Skip to content

Fix CodeQL security issues #7985

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

Merged
joaosaffran merged 9 commits into from
Dec 10, 2025
Merged

Fix CodeQL security issues #7985

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
a140517
adding fix for codeql issues
joaosaffran Dec 5, 2025
5a9b8bd
remove capture cmd
joaosaffran Dec 5, 2025
e97c64b
improving code formating
joaosaffran Dec 5, 2025
e5ab16c
add codeql comment to disable scanner
joaosaffran Dec 8, 2025
10136e6
add codeql comment to disable analysis
joaosaffran Dec 8, 2025
be1a5ab
add codeql comment to disable scanner
joaosaffran Dec 8, 2025
ebf740d
address comments from damyan
joaosaffran Dec 8, 2025
56722ba
apply clang format
joaosaffran-zz Dec 9, 2025
7d4247e
remove redundant assert
joaosaffran-zz Dec 9, 2025
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
8 changes: 6 additions & 2 deletions include/dxc/Support/dxcapi.use.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ class SpecificDllLoader : public DllLoader {
return S_OK;

#ifdef _WIN32
m_dll = LoadLibraryA(dllName);
m_dll = LoadLibraryA(
dllName); // CodeQL [SM01925] This is by design, intended to be used to
// test multiple validators versions.
if (m_dll == nullptr)
return HRESULT_FROM_WIN32(GetLastError());
m_createFn = (DxcCreateInstanceProc)GetProcAddress(m_dll, fnName);
Expand All @@ -81,7 +83,9 @@ class SpecificDllLoader : public DllLoader {
return hr;
}
#else
m_dll = ::dlopen(dllName, RTLD_LAZY);
m_dll = ::dlopen(
dllName, RTLD_LAZY); // CodeQL [SM01925] This is by design, intended to
// be used to test multiple validators versions.
if (m_dll == nullptr)
return E_FAIL;
m_createFn = (DxcCreateInstanceProc)::dlsym(m_dll, fnName);
Expand Down
4 changes: 2 additions & 2 deletions projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6393,8 +6393,8 @@ void DxbcConverter::EmitGSOutputRegisterStore(unsigned StreamId) {
CompType DxbcValueType = SE.GetCompType();
Type *pDxbcValueType = DxbcValueType.GetLLVMType(m_Ctx);

for (BYTE c = 0; c < SE.GetCols(); c++) {
BYTE Comp = SE.GetStartCol() + c;
for (unsigned c = 0; c < SE.GetCols(); c++) {
unsigned Comp = SE.GetStartCol() + c;

Value *pValue;
// 1. Load value from the corresponding temp reg.
Expand Down
4 changes: 3 additions & 1 deletion projects/dxilconv/lib/DxbcConverter/DxbcUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Type.h"
#include "llvm/Support/Casting.h"
#include <cassert>

#include "DxbcUtil.h"
#include "Support/DXIncludes.h"
Expand Down Expand Up @@ -46,7 +47,8 @@ CMask::CMask(BYTE StartComp, BYTE NumComp) {
(StartComp + NumComp - 1) < DXBC::kAllCompMask,
Copy link
Contributor

@tex3d tex3d Dec 8, 2025

Choose a reason for hiding this comment

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

I think this assert expression isn't correct:

  DXASSERT(StartComp < DXBC::kAllCompMask && NumComp <= DXBC::kAllCompMask &&
               (StartComp + NumComp - 1) < DXBC::kAllCompMask,

I think it should be this instead: (StartComp + NumComp) <= DXBC::kWidth

  DXASSERT(StartComp < DXBC::kWidth && NumComp <= DXBC::kWidth &&
               (StartComp + NumComp) <= DXBC::kWidth,

DXBC::kAllCompMask is 0x0F, which isn't the number of components available, but the mask with all components enabled.

With this corrected, I think @damyanp's comment below for having an assert like (StartComp + NumComp) < 256 is unnecessary.

"otherwise the caller did not check");
m_Mask = 0;
for (BYTE c = StartComp; c < StartComp + NumComp; c++) {
BYTE EndComp = StartComp + NumComp;
for (BYTE c = StartComp; c < EndComp; c++) {
m_Mask |= (1 << c);
}
}
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/unittests/HLSLExec/ExecutionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2480,7 +2480,7 @@ TEST_F(ExecutionTest, WaveIntrinsicsTest) {
}

// Waves should cover 4 threads or more.
LogCommentFmt(L"Found %u distinct lane ids: %u", firstLaneIds.size());
LogCommentFmt(L"Found %u distinct lane ids", firstLaneIds.size());
if (!dxbc) {
VERIFY_IS_GREATER_THAN_OR_EQUAL(values.size() / 4, firstLaneIds.size());
}
Expand Down
73 changes: 0 additions & 73 deletions tools/clang/utils/CaptureCmd
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion unittests/Support/ProgramTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ TEST(ProgramTest, TestWriteWithSystemEncoding) {
#else
char buf[10];
ASSERT_EQ(::read(fd, buf, 10), 10);
ASSERT_EQ(strncmp(buf, utf8_text, 10), 0);
ASSERT_EQ(strncmp(buf, utf8_text, 10),
0); // CodeQL [SM01932] the file content is controlled by the test.
#endif
::close(fd);
ASSERT_NO_ERROR(fs::remove(file_pathname.str()));
Expand Down
32 changes: 0 additions & 32 deletions utils/DSAclean.py
View file
Open in desktop

This file was deleted.