Skip to content

Commit d1d28a6

Browse files
damyanpCopilotgithub-actions[bot]
authored
Fix various errors found with UBSan (microsoft#8184)
* `SPIRVOptions.h` - one of the fields of this struct (signaturePacking) wasn't getting initialized before the struct's assignment operator was used. This resulted in the uninitialized fields being copied, which is UB. Although a more surgical fix is possible, it seemed better to just ensure all fields are initialized, setting a good precedent if any new fields are added. * `FrontendOptions.h` - uninitialized field * `APInt.cpp` - left shift of a signed integer * `Triple.h` - uninitialized field * `VirtualFileSystem.h` - uninitialized field * `Lookup.h` - uninitialized field * `MemoryBuffer.cpp` - avoid memcpy from a null dest (UB even if size is 0) Methodology: had copilot CLI get a baseline running the tests under ubsan, categorize the failures and fix them, and verify that gcc-14 and clang 18 are now ubsan clan. --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent b89799d commit d1d28a6

7 files changed

Lines changed: 59 additions & 51 deletions

File tree

include/dxc/Support/SPIRVOptions.h

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -39,40 +39,40 @@ enum class SpirvLayoutRule {
3939

4040
struct SpirvCodeGenOptions {
4141
/// Disable legalization and optimization and emit raw SPIR-V
42-
bool codeGenHighLevel;
43-
bool debugInfoFile;
44-
bool debugInfoLine;
45-
bool debugInfoSource;
46-
bool debugInfoTool;
47-
bool debugInfoRich;
42+
bool codeGenHighLevel = false;
43+
bool debugInfoFile = false;
44+
bool debugInfoLine = false;
45+
bool debugInfoSource = false;
46+
bool debugInfoTool = false;
47+
bool debugInfoRich = false;
4848
/// Use NonSemantic.Vulkan.DebugInfo.100 debug info instead of
4949
/// OpenCL.DebugInfo.100
50-
bool debugInfoVulkan;
51-
bool defaultRowMajor;
52-
bool disableValidation;
53-
bool enable16BitTypes;
54-
bool finiteMathOnly;
55-
bool enableReflect;
56-
bool invertY; // Additive inverse
57-
bool invertW; // Multiplicative inverse
58-
bool noWarnEmulatedFeatures;
59-
bool noWarnIgnoredFeatures;
60-
bool preserveBindings;
61-
bool preserveInterface;
62-
bool useDxLayout;
63-
bool useGlLayout;
64-
bool useLegacyBufferMatrixOrder;
65-
bool useScalarLayout;
66-
bool flattenResourceArrays;
67-
bool reduceLoadSize;
68-
bool autoShiftBindings;
69-
bool supportNonzeroBaseInstance;
70-
bool supportNonzeroBaseVertex;
71-
bool fixFuncCallArguments;
72-
bool enableMaximalReconvergence;
73-
bool useVulkanMemoryModel;
74-
bool useUnknownImageFormat;
75-
bool IEEEStrict;
50+
bool debugInfoVulkan = false;
51+
bool defaultRowMajor = false;
52+
bool disableValidation = false;
53+
bool enable16BitTypes = false;
54+
bool finiteMathOnly = false;
55+
bool enableReflect = false;
56+
bool invertY = false; // Additive inverse
57+
bool invertW = false; // Multiplicative inverse
58+
bool noWarnEmulatedFeatures = false;
59+
bool noWarnIgnoredFeatures = false;
60+
bool preserveBindings = false;
61+
bool preserveInterface = false;
62+
bool useDxLayout = false;
63+
bool useGlLayout = false;
64+
bool useLegacyBufferMatrixOrder = false;
65+
bool useScalarLayout = false;
66+
bool flattenResourceArrays = false;
67+
bool reduceLoadSize = false;
68+
bool autoShiftBindings = false;
69+
bool supportNonzeroBaseInstance = false;
70+
bool supportNonzeroBaseVertex = false;
71+
bool fixFuncCallArguments = false;
72+
bool enableMaximalReconvergence = false;
73+
bool useVulkanMemoryModel = false;
74+
bool useUnknownImageFormat = false;
75+
bool IEEEStrict = false;
7676
/// Maximum length in words for the OpString literal containing the shader
7777
/// source for DebugSource and DebugSourceContinued. If the source code length
7878
/// is larger than this number, we will use DebugSourceContinued instructions
@@ -81,14 +81,14 @@ struct SpirvCodeGenOptions {
8181
/// limitation of a single SPIR-V instruction size (0xFFFF) - 2 operand words
8282
/// for OpString. Currently a smaller value is only used to test
8383
/// DebugSourceContinued generation.
84-
uint32_t debugSourceLen;
85-
SpirvLayoutRule cBufferLayoutRule;
86-
SpirvLayoutRule sBufferLayoutRule;
87-
SpirvLayoutRule tBufferLayoutRule;
88-
SpirvLayoutRule ampPayloadLayoutRule;
84+
uint32_t debugSourceLen = 0;
85+
SpirvLayoutRule cBufferLayoutRule = SpirvLayoutRule::Void;
86+
SpirvLayoutRule sBufferLayoutRule = SpirvLayoutRule::Void;
87+
SpirvLayoutRule tBufferLayoutRule = SpirvLayoutRule::Void;
88+
SpirvLayoutRule ampPayloadLayoutRule = SpirvLayoutRule::Void;
8989
llvm::StringRef stageIoOrder;
9090
llvm::StringRef targetEnv;
91-
uint32_t maxId;
91+
uint32_t maxId = 0;
9292
llvm::SmallVector<int32_t, 4> bShift;
9393
llvm::SmallVector<int32_t, 4> sShift;
9494
llvm::SmallVector<int32_t, 4> tShift;
@@ -109,9 +109,11 @@ struct SpirvCodeGenOptions {
109109
std::optional<BindingInfo> samplerHeapBinding;
110110
std::optional<BindingInfo> counterHeapBinding;
111111

112-
bool signaturePacking; ///< Whether signature packing is enabled or not
112+
bool signaturePacking =
113+
false; ///< Whether signature packing is enabled or not
113114

114-
bool printAll; // Dump SPIR-V module before each pass and after the last one.
115+
bool printAll =
116+
false; // Dump SPIR-V module before each pass and after the last one.
115117

116118
// String representation of all command line options and input file.
117119
std::string clOptions;

include/llvm/ADT/Triple.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ class Triple {
193193
ArchType Arch;
194194

195195
/// The parsed subarchitecture type.
196-
SubArchType SubArch;
196+
SubArchType SubArch = NoSubArch;
197197

198198
/// The parsed vendor type.
199199
VendorType Vendor;

lib/Support/APInt.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -552,8 +552,10 @@ bool APInt::ult(const APInt& RHS) const {
552552
bool APInt::slt(const APInt& RHS) const {
553553
assert(BitWidth == RHS.BitWidth && "Bit widths must be same for comparison");
554554
if (isSingleWord()) {
555-
int64_t lhsSext = (int64_t(VAL) << (64-BitWidth)) >> (64-BitWidth);
556-
int64_t rhsSext = (int64_t(RHS.VAL) << (64-BitWidth)) >> (64-BitWidth);
555+
int64_t lhsSext =
556+
int64_t(uint64_t(VAL) << (64 - BitWidth)) >> (64 - BitWidth);
557+
int64_t rhsSext =
558+
int64_t(uint64_t(RHS.VAL) << (64 - BitWidth)) >> (64 - BitWidth);
557559
return lhsSext < rhsSext;
558560
}
559561

@@ -1061,8 +1063,8 @@ APInt APInt::ashr(unsigned shiftAmt) const {
10611063
return APInt(BitWidth, 0); // undefined
10621064
else {
10631065
unsigned SignBit = APINT_BITS_PER_WORD - BitWidth;
1064-
return APInt(BitWidth,
1065-
(((int64_t(VAL) << SignBit) >> SignBit) >> shiftAmt));
1066+
return APInt(BitWidth, (((int64_t(uint64_t(VAL) << SignBit) >> SignBit) >>
1067+
shiftAmt)));
10661068
}
10671069
}
10681070

lib/Support/MemoryBuffer.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,9 @@ MemoryBuffer::getMemBufferCopy(StringRef InputData, const Twine &BufferName) {
126126
getNewUninitMemBuffer(InputData.size(), BufferName);
127127
if (!Buf)
128128
return nullptr;
129-
memcpy(const_cast<char*>(Buf->getBufferStart()), InputData.data(),
130-
InputData.size());
129+
if (InputData.size())
130+
memcpy(const_cast<char *>(Buf->getBufferStart()), InputData.data(),
131+
InputData.size());
131132
return Buf;
132133
}
133134

tools/clang/include/clang/Basic/VirtualFileSystem.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,13 @@ class Status {
4040
llvm::sys::fs::perms Perms;
4141

4242
public:
43-
bool IsVFSMapped; // FIXME: remove when files support multiple names
43+
bool IsVFSMapped = false; // FIXME: remove when files support multiple names
4444

4545
public:
46-
Status() : Type(llvm::sys::fs::file_type::status_error) {}
46+
Status()
47+
: User(0), Group(0), Size(0),
48+
Type(llvm::sys::fs::file_type::status_error),
49+
Perms(llvm::sys::fs::perms_not_known) {}
4750
Status(const llvm::sys::fs::file_status &Status);
4851
Status(StringRef Name, StringRef RealName, llvm::sys::fs::UniqueID UID,
4952
llvm::sys::TimeValue MTime, uint32_t User, uint32_t Group,

tools/clang/include/clang/Frontend/FrontendOptions.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ class FrontendInputFile {
9292
bool IsSystem;
9393

9494
public:
95-
FrontendInputFile() : Buffer(nullptr), Kind(IK_None) { }
95+
FrontendInputFile() : Buffer(nullptr), Kind(IK_None), IsSystem(false) {}
9696
FrontendInputFile(StringRef File, InputKind Kind, bool IsSystem = false)
9797
: File(File.str()), Buffer(nullptr), Kind(Kind), IsSystem(IsSystem) { }
9898
FrontendInputFile(llvm::MemoryBuffer *buffer, InputKind Kind,

tools/clang/include/clang/Sema/Lookup.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ class LookupResult {
654654

655655
// Results.
656656
LookupResultKind ResultKind;
657-
AmbiguityKind Ambiguity; // ill-defined unless ambiguous
657+
AmbiguityKind Ambiguity = {}; // ill-defined unless ambiguous
658658
UnresolvedSet<8> Decls;
659659
CXXBasePaths *Paths;
660660
CXXRecordDecl *NamingClass;

0 commit comments

Comments
 (0)