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 Clang sanitizers on Windows #17475

Merged
merged 2 commits into from
Jan 17, 2025
Merged

Fix Clang sanitizers on Windows #17475

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07d404a
Fix Clang lib folder detection on Windows
cmb69 Jan 14, 2025
79af314
Suppress UB warnings regarding indirect function calls
cmb69 Jan 14, 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
42 changes: 18 additions & 24 deletions win32/build/confutils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3137,7 +3137,7 @@ function toolset_get_compiler_name(short)
var command = 'cmd /c ""' + PHP_CL + '" -v"';
var full = execute(command + '" 2>&1"');

return full.split(/\n/)[0].replace(/\s/g, ' ');
return trim(full.split(/\n/)[0].replace(/\s/g, ' '));
}

WARNING("Unsupported toolset");
Expand Down Expand Up @@ -3698,31 +3698,31 @@ function check_binary_tools_sdk()
function get_clang_lib_dir()
{
var ret = null;
var ver = null;
var ver = null, major = null;

if (COMPILER_NAME_LONG.match(/clang version ([\d\.]+) \((.*)\)/)) {
if (COMPILER_NAME_LONG.match(/clang version ((\d+)\.[\d\.]+)/)) {
ver = RegExp.$1;
major = RegExp.$2;
} else {
ERROR("Failed to determine clang lib path");
ERROR("Failed to determine clang version");
}

if (TARGET_ARCH != 'x86') {
ret = PROGRAM_FILES + "\\LLVM\\lib\\clang\\" + ver + "\\lib";
var cmd = "cmd /c where clang.exe";
Copy link
Member

Choose a reason for hiding this comment

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

was wondering if llvm-config.exe exists/is well supported on windows but maybe not too useful in those cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I wasn't aware of this, but apparently it is still not shipped with Windows binaries (see https://stackoverflow.com/questions/17096804/where-is-llvm-config-in-windows).

var path = trim(execute(cmd).split(/\n/)[0]).replace(/bin\\clang\.exe$/, "");
if (!FSO.FolderExists(path)) {
ERROR("Failed to determine clang installation folder");
}

ret = path + "lib\\clang\\" + major + "\\lib\\";
if (!FSO.FolderExists(ret)) {
ret = path + "lib\\clang\\" + ver + "\\lib\\";
if (!FSO.FolderExists(ret)) {
ret = null;
}
} else {
ret = PROGRAM_FILESx86 + "\\LLVM\\lib\\clang\\" + ver + "\\lib";
if (!FSO.FolderExists(ret)) {
ret = PROGRAM_FILES + "\\LLVM\\lib\\clang\\" + ver + "\\lib";
if (!FSO.FolderExists(ret)) {
ret = null;
}
}
}

if (null == ret) {
ERROR("Invalid clang lib path encountered");
ERROR("Failed to determine clang lib folder");
}

return ret;
Expand All @@ -3731,16 +3731,10 @@ function get_clang_lib_dir()
function add_asan_opts(cflags_name, libs_name, ldflags_name)
{

var ver = null;

if (COMPILER_NAME_LONG.match(/clang version ([\d\.]+) \((.*)\)/)) {
ver = RegExp.$1;
} else {
ERROR("Failed to determine clang lib path");
}
var lib_dir = get_clang_lib_dir();

if (!!cflags_name) {
ADD_FLAG(cflags_name, "-fsanitize=address,undefined");
ADD_FLAG(cflags_name, "-fsanitize=address,undefined -fno-sanitize=function");
}
if (!!libs_name) {
if (TARGET_ARCH == 'x64') {
Expand All @@ -3754,7 +3748,7 @@ function add_asan_opts(cflags_name, libs_name, ldflags_name)
}

if (!!ldflags_name) {
ADD_FLAG(ldflags_name, "/libpath:\"" + get_clang_lib_dir() + "\\windows\"");
ADD_FLAG(ldflags_name, "/libpath:\"" + lib_dir + "\\windows\"");
}
}

Expand Down
Loading