Skip to content

[compiler-rt][ASan] Work around msvc cl x64 build warnings #156887

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+2 −1
Draft

[compiler-rt][ASan] Work around msvc cl x64 build warnings #156887

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion compiler-rt/lib/asan/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ set(ASAN_CFLAGS ${SANITIZER_COMMON_CFLAGS})

# Win/ASan relies on the runtime functions being hotpatchable. See
# https://github.com/llvm/llvm-project/pull/149444
if(MSVC)
# MSVC cl 64 bit architectures treat /hotpatch as an unknown option
if(CLANG_CL OR (MSVC AND "i386" IN_LIST ASAN_SUPPORTED_ARCH))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, it doesn't seem like the CLANG_CL check actually works; at least not in the "runtimes build" (-DLLVM_ENABLE_RUNTIMES=compiler-rt). For example, I'm getting warnings about the /experimental: flags (line 124) when building with clang-cl.

It would be great if someone fixed that of course :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh no 😃 thanks for pointing that out!

I'll look into what it would take to get that online. In the meantime, I'll mark this back as a draft.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CLANG_CL is also used a few lines down to silence some other warnings related to MSVC. I'm wondering if I have something locally that is precluding me from seeing the same issues you are.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Those are the ones I meant:

llvm-project/compiler-rt/lib/asan/CMakeLists.txt

Lines 121 to 124 in b2ff3e7

# Silence warnings in system headers with MSVC.
if(NOT CLANG_CL)
append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external;/external:W0;/external:anglebrackets" ASAN_CFLAGS)
endif()

I don't think they work, i.e. the /experimental:external flags get passed also when using clang-cl, and cause warnings, e.g. from https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8704672462688910449/+/u/package_clang/stdout

 [112/349] Building CXX object compiler-rt\lib\asan\CMakeFiles\RTAsan_dynamic.x86_64.dir\asan_debugging.cpp.obj
 clang-cl: warning: argument unused during compilation: '/experimental:external' [-Wunused-command-line-argument]
 clang-cl: warning: argument unused during compilation: '/external:anglebrackets' [-Wunused-command-line-argument]

I think maybe the CLANG_CL check used to work when building with -DLLVM_ENABLE_PROJECTS=compiler-rt but not in the new "runtimes build" using -DLLVM_ENABLE_RUNTIMES=compiler-rt but that's just me speculating.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would help if I could read 🤣
Ok, thanks for the clue into where to check!

Copy link
Contributor

Choose a reason for hiding this comment

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

Related to this, I left a comment on this thread about "runtime build" issues: #58568

list(APPEND ASAN_CFLAGS /hotpatch)
endif()

Expand Down