-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
@RussKeldorph please review |
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Ubuntu gcstress0xc |
@RussKeldorph Microsoft.NETCore.CoreDisTools v1.0.1-prerelease-00001 is already pushed to https://dotnet.myget.org/gallery/dotnet-core |
const uint8_t *Bytes1, size_t Size1, | ||
const uint8_t *Address2, const uint8_t *Bytes2, | ||
size_t Size2); | ||
typedef bool NearDiffCodeBlocks_Sig(const CorAsmDiff *AsmDiff, |
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 realize I missed this before, but I prefer a _t suffix over _Sig, but it's not a big deal. Also, you can use the signature type to declare the function like "DllIFace NearDifferCodeBlocks_Sig NearDifferCodeBlocks;" to save duplication. You don't have to change either of these, though.
LGTM aside from minor comments and assuming the tests should be passing. |
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Ubuntu gcstress0xc |
@dotnet-bot test this please |
Failure in Windows GCStress is because of https://github.com/dotnet/coreclr/issues/3925 |
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Ubuntu gcstress0xc |
@dotnet-bot test this please |
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Ubuntu gcstress0xc |
1 similar comment
@dotnet-bot test Ubuntu gcstress0xc |
@dotnet-bot test Windows_NT gcstress0xc |
1 similar comment
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test this please |
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Ubuntu gcstress0xc |
CoreCLR uses the CoreDisTools library for utilities like GCStress and SuperPMI. To aid these utilities, this change adds the CoreDisTools header to CoreCLR repository. This cleans up duplication of type/enum definitions in CoreDisTools and CoreCLR sources, and provides for a cleaner way to use the interface. The understanding here is that CoreCLR defines the required CoreDisTools interface, and will publish the header into the cmake-install directory. Any repository can pick up this header and supply implementations for the imported functions. [Currently the only implementation is the one that uses LLVM disassembler in the LLILC tree]. Two other changes: 1) Define TypeDefs for exported function types in cordistools.h to avoid duplicating the signature in importing code. 2) Move to the libcoredistools version 1.0.1, and use the new interface in disassembler.cpp
@dotnet-bot test Windows_NT gcstress0xc |
@dotnet-bot test Ubuntu gcstress0xc |
The Windows_NT x64 Checked Build and Test (Jit - GCStress=0xC) failure is not related to the disassembler. CoreDisTools is unpacked and loaded OK. |
OSX x64 Checked Build and Test failure is also a test failure — unrelated to CoreDisTOols |
Add CoreDisTools header Commit migrated from dotnet/coreclr@4472a22
CoreCLR uses the CoreDisTools library for utilities like GCStress
and SuperPMI. To aid these utilities, this change adds the
CoreDisTools header to CoreCLR repository.
This cleans up duplication of type/enum definitions in CoreDisTools
and CoreCLR sources, and provides for a cleaner way to use the
interface.
The understanding here is that CoreCLR defines the required
CoreDisTools interface, and will publish the header into the
cmake-install directory.
Any repository can pick up this header and supply implementations
for the imported functions. [Currently the only implementation
is the one that uses LLVM disassembler in the LLILC tree].
Two other changes:
to avoid duplicating the signature in importing code.
interface in disassembler.cpp