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.

Sign up for GitHub

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

Fix build error after update the SDK submodule in BinSkim #915

Merged
merged 9 commits into from
Dec 8, 2023
Merged

Fix build error after update the SDK submodule in BinSkim #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
329ed88
Fix update Submodule error
shaopeng-gh May 31, 2023
2669700
Merge branch 'main' into users/shaopeng-gh/fixupdatesubmodule_ok
shaopeng-gh Jun 2, 2023
539a4e5
Merge branch 'main' into users/shaopeng-gh/fixupdatesubmodule_ok
shaopeng-gh Jun 6, 2023
1252971
merge from main
shaopeng-gh Jul 26, 2023
428c916
Merge branch 'main' into users/shaopeng-gh/fixupdatesubmodule_ok
shaopeng-gh Dec 5, 2023
74998fb
Update to Latest SDK
shaopeng-gh Dec 5, 2023
29a14fe
Update Release History
shaopeng-gh Dec 5, 2023
4e717ff
Fix casing for Dia2Lib
shaopeng-gh Dec 5, 2023
a1f113e
Format Release History Update `Sarif.Sdk` submodule
shaopeng-gh Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions NuGet.Config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<config>
<add key="globalPackagesFolder" value="./src/packages" />
Copy link
Collaborator Author

@shaopeng-gh shaopeng-gh May 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

we have similar settings in SDK with 2 settings, based on building and testing only this one is needed. #Closed

</config>
</configuration>
7 changes: 4 additions & 3 deletions ReleaseHistory.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- NEW => new feature

## UNRELEASED
* DEP: Update `Sarif.Sdk` submodule from [bc8cb57 to fd6e615](https://github.com/microsoft/sarif-sdk/compare/bc8cb57...fd6e615). Reference [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/fd6e615/ReleaseHistory.md).

Copy link
Member

Choose a reason for hiding this comment

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

This line is inconsistent with other entries in this file: they all need to be consistent. I think Stacy recommended removing the word Full, as below:

if that is correct, please update the entire release history to bring all of these DEP entries into conformance. Also someone needs to make sure our release notes maintenance documentation has the correct standard listed there.

Copy link
Member

Choose a reason for hiding this comment

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

Also, why exactly did you need to make this dependency update? It is a good practice to cite the specific incoming change that's critical to binskim.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think Stacy recommended removing the word Full --- The original recommendation from Stacy was to add Reference not removing full, I checked with her we both agree removing full looks good. i have updated.

why exactly did you need to make this dependency update? ---- Only needed is the change that we disable the warning for using Newton old versions 9. So that build will not just stop as error. This is only to fix our build process not related to the user.

## **v4.2.1**
* FPS: `BA2004.EnableSecureSourceCodeHashing` now will no longer generate false positives on precompiled headers, they are always without hash. [#965](https://github.com/microsoft/binskim/pull/965)
Expand All @@ -39,7 +40,7 @@
* NEW: Support `SymbolPath`, `LocalSymbolDirectories`, `IgnorePdbLoadError` option when using config file, in addtion to passing as command line parameters. [#944](https://github.com/microsoft/binskim/pull/944)

## **v4.1.0**
* DEP: Update Sarif.Sdk submodule from [120fae3 to bc8cb57](https://github.com/microsoft/sarif-sdk/compare/120fae3...bc8cb57). Full [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/bc8cb57/ReleaseHistory.md).
* DEP: Update `Sarif.Sdk` submodule from [120fae3 to bc8cb57](https://github.com/microsoft/sarif-sdk/compare/120fae3...bc8cb57). Reference [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/bc8cb57/ReleaseHistory.md).
* DEP: Upgrade ELFSharp from 2.16.1 to 2.17.1. [#872](https://github.com/microsoft/binskim/pull/872)
* BRK: Remove `--verbose` command-line option (in favor of `--level` and `--kind`). [#853](https://github.com/microsoft/binskim/pull/853)
* BRK: Remove `--hashes` command-line option (in favor of `--insert Hashes`). [#853](https://github.com/microsoft/binskim/pull/853)
Expand All @@ -53,7 +54,7 @@
* NEW: `CompilerInformation` telemetry now emits the last modified date of the PDB associated with the analyzed binary. [#871](https://github.com/microsoft/binskim/pull/871)

## **v4.0.0**
* DEP: Update Sarif.Sdk submodule from [fc9a9df to 2d52c53](https://github.com/microsoft/sarif-sdk/compare/fc9a9df...2d52c53). Full [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/2d52c53/ReleaseHistory.md).
* DEP: Update `Sarif.Sdk` submodule from [fc9a9df to 2d52c53](https://github.com/microsoft/sarif-sdk/compare/fc9a9df...2d52c53). Reference [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/2d52c53/ReleaseHistory.md).
* DEP: Upgrade `Elfsharp.2.16.0` to `Elfsharp.2.16.1`[#791](https://github.com/microsoft/binskim/pull/791)
* DEP: Upgrade BinSkim to .net6.0 as .net core 3.1 reached end of support on 12/13/2022.
* DEP: Upgrade `Newtonsoft.JSON` package to 13.0.2 to resolve security alert.
Expand All @@ -78,7 +79,7 @@
## **v1.9.5** [NuGet Package](https://www.nuget.org/packages/Microsoft.CodeAnalysis.BinSkim/1.9.5)
* DEP: Upgrade ELFSharp from 2.14.0 to 2.15.0. [#631](https://github.com/microsoft/binskim/pull/631)
* DEP: Upgrade System.Reflection.Metadata from 5.0.0 to 6.0.1 and System.Collections.Immutable from 5.0.0 to 6.0.0. [#605](https://github.com/microsoft/binskim/pull/605)
* DEP: Upgrade Sarif.Sdk by updating submodule from [4e9f606 to fc9a9df](https://github.com/microsoft/sarif-sdk/compare/4e9f606bb0e88428866e253352cdc70dc68f98cb...fc9a9dfb865096b5aaa9fa3651854670940f7459). [#638](https://github.com/microsoft/binskim/pull/638)
* DEP: Update `Sarif.Sdk` submodule from [4e9f606 to fc9a9df](https://github.com/microsoft/sarif-sdk/compare/4e9f606...fc9a9df). Reference [SARIF SDK Release History](https://github.com/microsoft/sarif-sdk/blob/fc9a9df/ReleaseHistory.md).
* NEW: Enable BinSkim for MacOS. [#576](https://github.com/microsoft/binskim/pull/576)
* FPR: Skip `BA2025.EnableShadowStack` rule for ARM Binaries which cannot use `/CETCOMPAT`. [#650](https://github.com/microsoft/binskim/pull/650)
* BUG: Fix missing `commandLineId` from `CommandLineInformation` event. [#652](https://github.com/microsoft/binskim/pull/652)
Expand Down
0 refs/dia2lib.dll → refs/Dia2Lib.dll
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions src/BinSkim.Driver/BinSkim.Driver.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
</ItemGroup>

<ItemGroup>
<Reference Include="dia2lib">
<HintPath>..\..\refs\dia2lib.dll</HintPath>
<Reference Include="Dia2Lib">
<HintPath>..\..\refs\Dia2Lib.dll</HintPath>
Copy link
Collaborator Author

@shaopeng-gh shaopeng-gh May 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The name changed is needed because in Linux it is case sensitive. #Resolved

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shaopeng-gh What exactly is the issue when the SARIF SDK Dia2Lib.dll is used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

misssing classes: IDiaEnumSegments, IDiaSectionContrib etc.
conflict classes: IDiaDataSource2, IDiaDataSource3
image

Copy link

Choose a reason for hiding this comment

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

If there isn't a reason not to change the Reference itself (as opposed to the hint), can you please change it to agree with the hint?

Copy link

Choose a reason for hiding this comment

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

Same comment for lines below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated the
<Reference Include="dia2lib">
to
<Reference Include="Dia2Lib">.
I have searched the whole repo there is 0 lower case of dia2lib.

<EmbedInteropTypes>False</EmbedInteropTypes>
</Reference>
</ItemGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/BinSkim.Rules/BinSkim.Rules.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
</PropertyGroup>

<ItemGroup>
<Reference Include="dia2lib">
<HintPath>..\..\refs\dia2lib.dll</HintPath>
<Reference Include="Dia2Lib">
<HintPath>..\..\refs\Dia2Lib.dll</HintPath>
<EmbedInteropTypes>False</EmbedInteropTypes>
</Reference>
</ItemGroup>
Expand Down
16 changes: 13 additions & 3 deletions src/BinaryParsers/BinaryParsers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
</ItemGroup>

<ItemGroup>
<Reference Include="dia2lib">
<HintPath>..\..\refs\dia2lib.dll</HintPath>
<Reference Include="Dia2Lib">
<HintPath>..\..\refs\Dia2Lib.dll</HintPath>
<EmbedInteropTypes>False</EmbedInteropTypes>
</Reference>
</ItemGroup>
Expand All @@ -43,4 +43,14 @@
</EmbeddedResource>
</ItemGroup>

</Project>
<PropertyGroup>
<PreBuildEvent Condition="$(OS) == 'Windows_NT'">
echo Copying file from "$(MSBuildProjectDirectory)\..\..\refs\Dia2Lib.dll" to "$(MSBuildProjectDirectory)\..\..\src\packages\microsoft.diagnostics.tracing.traceevent\3.1.3\lib\netstandard2.0\"
copy "$(MSBuildProjectDirectory)\..\..\refs\Dia2Lib.dll" "$(MSBuildProjectDirectory)\..\..\src\packages\microsoft.diagnostics.tracing.traceevent\3.1.3\lib\netstandard2.0\"
Copy link
Collaborator Author

@shaopeng-gh shaopeng-gh May 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is to replace with our version of Dia2Lib.dll that works with our code. #Closed

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shaopeng-gh I am not sure this is the best fix here. So you are essentially hard coding the Dia2Lib.dll to use? From the PR description, it seems like this dll is used for the new events model. Would hard-coding our version of the dll affect the generation of events?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is good point if it will break the generation of events.
I am not familiar about it,
I see the code we are doing check like:
if (!string.IsNullOrEmpty(globalContext.EventsFilePath))
to enable generation of events, and the setting of Context.EventsFilePath is:
context.EventsFilePath = Environment.GetEnvironmentVariable("SPMI_ETW") ?? options.EventsFilePath ?? context.EventsFilePath;
Is this only enabled in SPMI?

</PreBuildEvent>
<PreBuildEvent Condition="$(OS) != 'Windows_NT'">
echo Copying file from "$(MSBuildProjectDirectory)\..\..\refs\Dia2Lib.dll" to "$(MSBuildProjectDirectory)\..\..\src\packages\microsoft.diagnostics.tracing.traceevent\3.1.3\lib\netstandard2.0\"
cp -f "$(MSBuildProjectDirectory)\..\..\refs\Dia2Lib.dll" "$(MSBuildProjectDirectory)\..\..\src\packages\microsoft.diagnostics.tracing.traceevent\3.1.3\lib\netstandard2.0\"
</PreBuildEvent>
</PropertyGroup>
</Project>
2 changes: 1 addition & 1 deletion src/Test.UnitTests.BinaryParsers/Test.UnitTests.BinaryParsers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

<ItemGroup>
<Reference Include="Dia2Lib">
<HintPath>..\..\refs\dia2lib.dll</HintPath>
<HintPath>..\..\refs\Dia2Lib.dll</HintPath>
</Reference>
</ItemGroup>

Expand Down
Empty file added src/packages/.gitkeep
View file
Open in desktop
Empty file.
2 changes: 1 addition & 1 deletion src/sarif-sdk
Open in desktop
Submodule sarif-sdk updated 180 files
Loading