This is an archive of the discontinued LLVM Phabricator instance.

Differential D116872

[compiler-rt] Silence warnings when building with MSVC
ClosedPublic

Authored by aganea on Jan 8 2022, 3:26 PM.

Details

Reviewers
dblaikie
hans
rnk
vitalybuka
Commits
rG262e2886b842: [compiler-rt] Fix MSVC `/external` detection in cmake scripts
rGae4c643bcdf2: [compiler-rt] Silence warnings when building with MSVC
Summary

It's been a while since I'm seeing all sorts of warnings when building with MSVC 2019 (some of them below):

[1921/4575] Building CXX object projects\compiler-rt\lib\s...rCommonSymbolizer.x86_64.dir\sanitizer_stack_store.cpp.obj
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_stack_store.cpp(241): warning C4200: nonstandard extension used: zero-sized array in struct/union
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_stack_store.cpp(241): note: This member will be ignored by a defaulted constructor or copy/move assignment operator
[2110/4575] Building C object projects\compiler-rt\lib\builtins\CMakeFiles\clang_rt.builtins-x86_64.dir\udivmoddi4.c.objD:\git\llvm-project\compiler-rt\lib\builtins\udivmoddi4.c(62) : warning C4723: potential divide by 0
[2114/4575] Building C object projects\compiler-rt\lib\builtins\CMakeFiles\clang_rt.builtins-x86_64.dir\cpu_model.c.obj
D:\git\llvm-project\compiler-rt\lib\builtins\cpu_model.c(18): warning C4067: unexpected tokens following preprocessor directive - expected a newline
[2158/4575] Building C object projects\compiler-rt\lib\builtins\CMakeFiles\clang_rt.builtins-x86_64.dir\emutls.c.obj
D:\git\llvm-project\compiler-rt\lib\builtins\emutls.c(39): warning C4200: nonstandard extension used: zero-sized array in struct/union
[2179/4575] Building CXX object projects\compiler-rt\lib\u...keFiles\RTUbsan_cxx.x86_64.dir\ubsan_type_hash_win.cpp.obj
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(51): warning C4005: 'INT32_MIN': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(365): note: see previous definition of 'INT32_MIN'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(52): warning C4005: 'INT64_MIN': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(371): note: see previous definition of 'INT64_MIN'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(55): warning C4005: 'INT32_MAX': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(367): note: see previous definition of 'INT32_MAX'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(56): warning C4005: 'INT64_MAX': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(373): note: see previous definition of 'INT64_MAX'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(59): warning C4005: 'UINT32_MAX': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(369): note: see previous definition of 'UINT32_MAX'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(60): warning C4005: 'UINT64_MAX': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(375): note: see previous definition of 'UINT64_MAX'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\stdint.h(91): warning C4005: 'UINTPTR_MAX': macro redefinition
D:\git\llvm-project\compiler-rt\lib\sanitizer_common\sanitizer_internal_defs.h(378): note: see previous definition of 'UINTPTR_MAX']

After this patch, there's no more warnings. Tested in various configs, with clang-cl and MSVC.

Diff Detail

Event Timeline

aganea created this revision.Jan 8 2022, 3:26 PM
Herald added subscribers: pengfei, mgorny, dberris. · View Herald TranscriptJan 8 2022, 3:26 PM
aganea requested review of this revision.Jan 8 2022, 3:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 8 2022, 3:26 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
aganea retitled this revision from [compiler-rt] Silence all warnings when building with MSVC to [compiler-rt] Silence warnings when building with MSVC.Jan 8 2022, 3:26 PM
Harbormaster completed remote builds in B142255: Diff 398380.Jan 8 2022, 3:49 PM
hans added inline comments.Jan 10 2022, 1:09 AM
compiler-rt/lib/asan/CMakeLists.txt
95

I thought /external: is only available in recent MSVC versions. Do we need to guard this on the version or check whether it's available?

compiler-rt/lib/profile/CMakeLists.txt
125 ↗(On Diff #398380)

Just above it seems we're checking if a /wd flag is supported before applying it. Do we need to do that here also?

vitalybuka added inline comments.Jan 10 2022, 5:39 PM
compiler-rt/lib/builtins/cpu_model.c
16 ↗(On Diff #398380)

this change looks unrelated to the patch

compiler-rt/lib/builtins/emutls.c
33 ↗(On Diff #398380)

why we don't need 4206 any more?

compiler-rt/lib/builtins/udivmoddi4.c
24 ↗(On Diff #398380)
aganea updated this revision to Diff 398940.Jan 11 2022, 7:18 AM
aganea marked 2 inline comments as done.

As suggested by @hans

hans accepted this revision.Jan 11 2022, 7:21 AM

lgtm

This revision is now accepted and ready to land.Jan 11 2022, 7:21 AM
aganea marked 2 inline comments as done.Jan 11 2022, 7:35 AM
aganea added inline comments.
compiler-rt/lib/builtins/cpu_model.c
16 ↗(On Diff #398380)

MSVC doesn't have __has_attribute and we're checking that just below.

compiler-rt/lib/builtins/emutls.c
33 ↗(On Diff #398380)

Actually 4206 was unrelated, I think it was a mistake, 4200 is the warning that the comment above is describing.

warning C4206 (previous): nonstandard extension used : translation unit is empty

warning C4200 (current): nonstandard extension used: zero-sized array in struct/union

compiler-rt/lib/builtins/udivmoddi4.c
24 ↗(On Diff #398380)

Will fix.

Harbormaster completed remote builds in B142652: Diff 398940.Jan 11 2022, 7:36 AM
This revision was landed with ongoing or failed builds.Jan 11 2022, 7:37 AM
Closed by commit rGae4c643bcdf2: [compiler-rt] Silence warnings when building with MSVC (authored by aganea). · Explain Why
This revision was automatically updated to reflect the committed changes.
aganea marked 2 inline comments as done.
aganea added a commit: rGae4c643bcdf2: [compiler-rt] Silence warnings when building with MSVC.
ychen added a subscriber: ychen.Jan 14 2022, 7:03 PM

clang-cl begins to emit something like this

clang-cl: warning: argument unused during compilation: '/experimental:external' [-Wunused-command-line-argument]
clang-cl: warning: argument unused during compilation: '/external:W0' [-Wunused-command-line-argument]
clang-cl: warning: argument unused during compilation: '/external:anglebrackets' [-Wunused-command-line-argument]

Any suggestions to fix this? Thanks a lot.

aganea mentioned this in rG7fcc44bca4b8: [compiler-rt][cmake] Fix clang-cl warnings introduced in ae4c643bcdf2.Jan 15 2022, 9:06 AM
aganea added a comment.Jan 15 2022, 9:07 AM
In D116872#3245667, @ychen wrote:

clang-cl begins to emit something like this

clang-cl: warning: argument unused during compilation: '/experimental:external' [-Wunused-command-line-argument]
clang-cl: warning: argument unused during compilation: '/external:W0' [-Wunused-command-line-argument]
clang-cl: warning: argument unused during compilation: '/external:anglebrackets' [-Wunused-command-line-argument]

Any suggestions to fix this? Thanks a lot.

Thanks for the report. Fixed in rG7fcc44bca4b8d3326982692b30d8b6ea8a91db56. Please let me know if there's anything else!

ychen added a comment.Jan 15 2022, 10:09 AM
In D116872#3246130, @aganea wrote:
In D116872#3245667, @ychen wrote:

clang-cl begins to emit something like this

clang-cl: warning: argument unused during compilation: '/experimental:external' [-Wunused-command-line-argument]
clang-cl: warning: argument unused during compilation: '/external:W0' [-Wunused-command-line-argument]
clang-cl: warning: argument unused during compilation: '/external:anglebrackets' [-Wunused-command-line-argument]

Any suggestions to fix this? Thanks a lot.

Thanks for the report. Fixed in rG7fcc44bca4b8d3326982692b30d8b6ea8a91db56. Please let me know if there's anything else!

Thank you!

yurybura added a subscriber: yurybura.Sep 25 2023, 3:52 AM

@aganea

The following CMake code is incorrect due to append_list_if doesn't support multiple space separated flags:

append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external /external:W0 /external:anglebrackets" UBSAN_CXXFLAGS)

MSVC compiler generates warning:

cl : Command line warning D9002 : ignoring unknown option '/experimental:external /external:W0 /external:anglebrackets'

Possible fix:

append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external;/external:W0;/external:anglebrackets" UBSAN_CXXFLAGS)

or

append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG /experimental:external UBSAN_CXXFLAGS)
append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG /external:W0 UBSAN_CXXFLAGS)
append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG /external:anglebrackets UBSAN_CXXFLAGS)
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2023, 3:52 AM
Herald added a subscriber: Enna1. · View Herald Transcript
yurybura reopened this revision.Sep 26 2023, 1:39 PM
This revision is now accepted and ready to land.Sep 26 2023, 1:39 PM
Closed by commit rG262e2886b842: [compiler-rt] Fix MSVC `/external` detection in cmake scripts (authored by aganea). · Explain WhyOct 2 2023, 9:29 AM
This revision was automatically updated to reflect the committed changes.
aganea added a commit: rG262e2886b842: [compiler-rt] Fix MSVC `/external` detection in cmake scripts.

Revision Contents

PathSize
compiler-rt/
lib/
asan/
2 lines
interception/
2 lines
ubsan/
2 lines

Diff 557535

compiler-rt/lib/asan/CMakeLists.txt

Show First 20 Lines • Show All 86 Lines • ▼ Show 20 Lines
set(ASAN_CFLAGS ${SANITIZER_COMMON_CFLAGS}) set(ASAN_CFLAGS ${SANITIZER_COMMON_CFLAGS})
set(ASAN_COMMON_DEFINITIONS ${COMPILER_RT_ASAN_SHADOW_SCALE_DEFINITION}) set(ASAN_COMMON_DEFINITIONS ${COMPILER_RT_ASAN_SHADOW_SCALE_DEFINITION})
append_rtti_flag(OFF ASAN_CFLAGS) append_rtti_flag(OFF ASAN_CFLAGS)
# Silence warnings in system headers with MSVC. # Silence warnings in system headers with MSVC.
if(NOT CLANG_CL) if(NOT CLANG_CL)
append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external /external:W0 /external:anglebrackets" ASAN_CFLAGS) append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external;/external:W0;/external:anglebrackets" ASAN_CFLAGS)
hansUnsubmitted
Done
ReplyInline Actions

I thought /external: is only available in recent MSVC versions. Do we need to guard this on the version or check whether it's available?

hans: I thought /external: is only available in recent MSVC versions. Do we need to guard this on the…
endif() endif()
# Too many existing bugs, needs cleanup. # Too many existing bugs, needs cleanup.
append_list_if(COMPILER_RT_HAS_WNO_FORMAT -Wno-format ASAN_CFLAGS) append_list_if(COMPILER_RT_HAS_WNO_FORMAT -Wno-format ASAN_CFLAGS)
set(ASAN_DYNAMIC_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS}) set(ASAN_DYNAMIC_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS})
if(ANDROID) if(ANDROID)
▲ Show 20 Lines • Show All 275 Lines • Show Last 20 Lines

compiler-rt/lib/interception/CMakeLists.txt

Show All 15 Lines
include_directories(..) include_directories(..)
set(INTERCEPTION_CFLAGS ${SANITIZER_COMMON_CFLAGS}) set(INTERCEPTION_CFLAGS ${SANITIZER_COMMON_CFLAGS})
append_rtti_flag(OFF INTERCEPTION_CFLAGS) append_rtti_flag(OFF INTERCEPTION_CFLAGS)
# Silence warnings in system headers with MSVC. # Silence warnings in system headers with MSVC.
if(NOT CLANG_CL) if(NOT CLANG_CL)
append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external /external:W0 /external:anglebrackets" INTERCEPTION_CFLAGS) append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external;/external:W0;/external:anglebrackets" INTERCEPTION_CFLAGS)
endif() endif()
add_compiler_rt_object_libraries(RTInterception add_compiler_rt_object_libraries(RTInterception
OS ${SANITIZER_COMMON_SUPPORTED_OS} OS ${SANITIZER_COMMON_SUPPORTED_OS}
ARCHS ${SANITIZER_COMMON_SUPPORTED_ARCH} ARCHS ${SANITIZER_COMMON_SUPPORTED_ARCH}
SOURCES ${INTERCEPTION_SOURCES} SOURCES ${INTERCEPTION_SOURCES}
ADDITIONAL_HEADERS ${INTERCEPTION_HEADERS} ADDITIONAL_HEADERS ${INTERCEPTION_HEADERS}
CFLAGS ${INTERCEPTION_CFLAGS}) CFLAGS ${INTERCEPTION_CFLAGS})
if(COMPILER_RT_INCLUDE_TESTS) if(COMPILER_RT_INCLUDE_TESTS)
add_subdirectory(tests) add_subdirectory(tests)
endif() endif()

compiler-rt/lib/ubsan/CMakeLists.txt

Show First 20 Lines • Show All 51 Lines • ▼ Show 20 Lines
append_list_if(SANITIZER_CAN_USE_CXXABI -DUBSAN_CAN_USE_CXXABI UBSAN_STANDALONE_CFLAGS) append_list_if(SANITIZER_CAN_USE_CXXABI -DUBSAN_CAN_USE_CXXABI UBSAN_STANDALONE_CFLAGS)
set(UBSAN_CXXFLAGS ${SANITIZER_COMMON_CFLAGS}) set(UBSAN_CXXFLAGS ${SANITIZER_COMMON_CFLAGS})
append_rtti_flag(ON UBSAN_CXXFLAGS) append_rtti_flag(ON UBSAN_CXXFLAGS)
append_list_if(SANITIZER_CAN_USE_CXXABI -DUBSAN_CAN_USE_CXXABI UBSAN_CXXFLAGS) append_list_if(SANITIZER_CAN_USE_CXXABI -DUBSAN_CAN_USE_CXXABI UBSAN_CXXFLAGS)
# Silence warnings in system headers with MSVC. # Silence warnings in system headers with MSVC.
if(NOT CLANG_CL) if(NOT CLANG_CL)
append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external /external:W0 /external:anglebrackets" UBSAN_CXXFLAGS) append_list_if(COMPILER_RT_HAS_EXTERNAL_FLAG "/experimental:external;/external:W0;/external:anglebrackets" UBSAN_CXXFLAGS)
endif() endif()
set(UBSAN_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS}) set(UBSAN_LINK_FLAGS ${SANITIZER_COMMON_LINK_FLAGS})
set(UBSAN_DYNAMIC_LIBS set(UBSAN_DYNAMIC_LIBS
${COMPILER_RT_UNWINDER_LINK_LIBS} ${COMPILER_RT_UNWINDER_LINK_LIBS}
${SANITIZER_CXX_ABI_LIBRARIES} ${SANITIZER_CXX_ABI_LIBRARIES}
${SANITIZER_COMMON_LINK_LIBS}) ${SANITIZER_COMMON_LINK_LIBS})
▲ Show 20 Lines • Show All 211 Lines • Show Last 20 Lines