Skip to content

Commit d747bcc

Browse files
author
joaosaffran
committed
addressing PR comments and adding more tests
1 parent 330369a commit d747bcc

File tree

6 files changed

+104
-14
lines changed

6 files changed

+104
-14
lines changed

‎llvm/include/llvm/Object/DXContainer.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "llvm/Support/MemoryBufferRef.h"
2424
#include "llvm/TargetParser/Triple.h"
2525
#include <array>
26+
#include <cstddef>
2627
#include <variant>
2728

2829
namespace llvm {
@@ -192,9 +193,9 @@ class RootSignature {
192193
DataSize = sizeof(dxbc::RootConstants);
193194
break;
194195
}
195-
auto EndOfSectionByte = getNumStaticSamplers() == 0
196-
? PartData.size()
197-
: getStaticSamplersOffset();
196+
size_t EndOfSectionByte = getNumStaticSamplers() == 0
197+
? PartData.size()
198+
: getStaticSamplersOffset();
198199

199200
if (Header.ParameterOffset + DataSize > EndOfSectionByte)
200201
return parseFailed("Reading structure out of file bounds");

‎llvm/lib/MC/DXContainerRootSignature.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ size_t RootSignatureDesc::getSize() const {
3232
size_t Size = sizeof(dxbc::RootSignatureHeader) +
3333
Parameters.size() * sizeof(dxbc::RootParameterHeader);
3434

35-
for (const auto &P : Parameters) {
35+
for (const mcdxbc::RootParameter &P : Parameters) {
3636
switch (P.Header.ParameterType) {
37-
case static_cast<uint32_t>(dxbc::RootParameterType::Constants32Bit):
37+
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
3838
Size += sizeof(dxbc::RootConstants);
3939
break;
4040
}
@@ -60,7 +60,7 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
6060
support::endian::write(BOS, Flags, llvm::endianness::little);
6161

6262
SmallVector<uint32_t> ParamsOffsets;
63-
for (const auto &P : Parameters) {
63+
for (const mcdxbc::RootParameter &P : Parameters) {
6464
support::endian::write(BOS, P.Header.ParameterType,
6565
llvm::endianness::little);
6666
support::endian::write(BOS, P.Header.ShaderVisibility,
@@ -72,10 +72,10 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
7272
assert(NumParameters == ParamsOffsets.size());
7373
for (size_t I = 0; I < NumParameters; ++I) {
7474
rewriteOffsetToCurrentByte(BOS, ParamsOffsets[I]);
75-
const auto &P = Parameters[I];
75+
const mcdxbc::RootParameter &P = Parameters[I];
7676

7777
switch (P.Header.ParameterType) {
78-
case static_cast<uint32_t>(dxbc::RootParameterType::Constants32Bit):
78+
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
7979
support::endian::write(BOS, P.Constants.ShaderRegister,
8080
llvm::endianness::little);
8181
support::endian::write(BOS, P.Constants.RegisterSpace,

‎llvm/lib/ObjectYAML/DXContainerEmitter.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,7 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
276276
Param.Type, Param.Visibility, Param.Offset};
277277

278278
switch (Param.Type) {
279-
280-
case static_cast<uint32_t>(dxbc::RootParameterType::Constants32Bit):
279+
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
281280
NewParam.Constants.Num32BitValues = Param.Constants.Num32BitValues;
282281
NewParam.Constants.RegisterSpace = Param.Constants.RegisterSpace;
283282
NewParam.Constants.ShaderRegister = Param.Constants.ShaderRegister;

‎llvm/lib/ObjectYAML/DXContainerYAML.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "llvm/ObjectYAML/DXContainerYAML.h"
15+
#include "llvm/ADT/STLForwardCompat.h"
1516
#include "llvm/ADT/ScopeExit.h"
1617
#include "llvm/BinaryFormat/DXContainer.h"
1718
#include "llvm/Support/Error.h"
@@ -45,7 +46,7 @@ DXContainerYAML::RootSignatureYamlDesc::create(
4546
RootSigDesc.RootParametersOffset = Data.getRootParametersOffset();
4647

4748
uint32_t Flags = Data.getFlags();
48-
for (const auto &PH : Data.param_headers()) {
49+
for (const dxbc::RootParameterHeader &PH : Data.param_headers()) {
4950

5051
RootParameterYamlDesc NewP;
5152
NewP.Offset = PH.ParameterOffset;
@@ -84,7 +85,8 @@ DXContainerYAML::RootSignatureYamlDesc::create(
8485
RootSigDesc.Parameters.push_back(NewP);
8586
}
8687
#define ROOT_ELEMENT_FLAG(Num, Val) \
87-
RootSigDesc.Val = (Flags & (uint32_t)dxbc::RootElementFlag::Val) > 0;
88+
RootSigDesc.Val = \
89+
(Flags & llvm::to_underlying(dxbc::RootElementFlag::Val)) > 0;
8890
#include "llvm/BinaryFormat/DXContainerConstants.def"
8991
return RootSigDesc;
9092
}
@@ -282,7 +284,7 @@ void MappingTraits<llvm::DXContainerYAML::RootParameterYamlDesc>::mapping(
282284
IO.mapRequired("ShaderVisibility", P.Visibility);
283285

284286
switch (P.Type) {
285-
case static_cast<uint32_t>(dxbc::RootParameterType::Constants32Bit):
287+
case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit):
286288
IO.mapRequired("Constants", P.Constants);
287289
break;
288290
}

‎llvm/unittests/Object/DXContainerTest.cpp‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,43 @@ TEST(DXCFile, MalformedSignature) {
822822
}
823823
}
824824

825+
TEST(RootSignature, MalformedData) {
826+
{
827+
// RootParametersOffset is 255.
828+
uint8_t Buffer[] = {
829+
0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
830+
0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
831+
0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
832+
0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
833+
0x01, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
834+
0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
835+
0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
836+
0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
837+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
838+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
839+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
840+
0x00};
841+
DXContainer C =
842+
llvm::cantFail(DXContainer::create(getMemoryBuffer<133>(Buffer)));
843+
844+
auto MaybeRS = C.getRootSignature();
845+
ASSERT_TRUE(MaybeRS.has_value());
846+
const auto &RS = MaybeRS.value();
847+
ASSERT_EQ(RS.getVersion(), 2u);
848+
ASSERT_EQ(RS.getNumParameters(), 1u);
849+
ASSERT_EQ(RS.getRootParametersOffset(), 255u);
850+
ASSERT_EQ(RS.getNumStaticSamplers(), 0u);
851+
ASSERT_EQ(RS.getStaticSamplersOffset(), 44u);
852+
ASSERT_EQ(RS.getFlags(), 17u);
853+
854+
// Since the offset is wrong, the data becomes invalid.
855+
auto RootParam = *RS.param_headers().begin();
856+
auto ParamView = RS.getParameter(RootParam);
857+
ASSERT_THAT_ERROR(ParamView.takeError(),
858+
FailedWithMessage("invalid parameter type"));
859+
}
860+
}
861+
825862
TEST(RootSignature, ParseRootFlags) {
826863
{
827864
uint8_t Buffer[] = {
@@ -844,7 +881,6 @@ TEST(RootSignature, ParseRootFlags) {
844881
ASSERT_EQ(RS->getStaticSamplersOffset(), 0u);
845882
ASSERT_EQ(RS->getFlags(), 0x01u);
846883
}
847-
848884
{
849885
// this parameter has the root signature definition missing some values.
850886
uint8_t Buffer[] = {

‎llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,58 @@ TEST(RootSignature, ParseRootFlags) {
148148
EXPECT_TRUE(memcmp(Buffer, Storage.data(), 68u) == 0);
149149
}
150150

151+
TEST(RootSignature, MalformedData) {
152+
SmallString<128> Storage;
153+
154+
// First read a fully explicit yaml with all sizes and offsets provided
155+
ASSERT_TRUE(convert(Storage, R"(--- !dxcontainer
156+
Header:
157+
Hash: [ 0x32, 0x9A, 0x53, 0xD8, 0xEC, 0xBE, 0x35, 0x6F, 0x5,
158+
0x39, 0xE1, 0xFE, 0x31, 0x20, 0xF0, 0xC1 ]
159+
Version:
160+
Major: 1
161+
Minor: 0
162+
FileSize: 133
163+
PartCount: 1
164+
PartOffsets: [ 36 ]
165+
Parts:
166+
- Name: RTS0
167+
Size: 89
168+
RootSignature:
169+
Version: 2
170+
NumRootParameters: 1
171+
RootParametersOffset: 255
172+
NumStaticSamplers: 0
173+
StaticSamplersOffset: 56
174+
Parameters:
175+
- ParameterType: 1
176+
ShaderVisibility: 2
177+
Constants:
178+
Num32BitValues: 16
179+
ShaderRegister: 15
180+
RegisterSpace: 14
181+
AllowInputAssemblerInputLayout: true
182+
DenyGeometryShaderRootAccess: true
183+
)"));
184+
185+
uint8_t Buffer[] = {
186+
0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
187+
0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
188+
0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
189+
0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
190+
0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
191+
0x00, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
192+
0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
193+
0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
194+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
195+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
196+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
197+
0x00};
198+
199+
EXPECT_EQ(Storage.size(), 133u);
200+
EXPECT_TRUE(memcmp(Buffer, Storage.data(), 133u) == 0);
201+
}
202+
151203
TEST(RootSignature, ParseRootConstants) {
152204
SmallString<128> Storage;
153205

0 commit comments

Comments
 (0)