Skip to content

Commit

Permalink
Add support for suppressing clang static analyzer issues with [[clang…
Browse files Browse the repository at this point in the history
…::suppress]]

https://bugs.webkit.org/show_bug.cgi?id=267981
<rdar://121489134>

Reviewed by Alex Christensen and Darin Adler.

This adds support for the `[[clang::suppress]]` attribute in clang-17.

Currently there is no way to ignore specific analyzer warnings, but the
macros add support for documenting the warning(s) in case support is
added later.

The `[[clang::suppress]]` attribute can be applied to a single line of
code or to a block of code.

* Source/WTF/wtf/Compiler.h:
(COMPILER_HAS_ATTRIBUTE): Add.
- Define helper macro since __has_attribute() is used more than once.
(COMPILER_HAS_CLANG_FEATURE):
- Update documentation link to https.
(FALLTHROUGH):
(NOT_TAIL_CALLED):
- Use COMPILER_HAS_ATTRIBUTE().
(IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_ATTRIBUTE): Add.
(IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_BEGIN): Add.
(IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_END): Add.
- Add support for the [[clang::suppress]] attribute.
(IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE): Add.
(TLS_MODEL_INITIAL_EXEC):
- Use COMPILER_HAS_ATTRIBUTE().

* Tools/TestWebKitAPI/Tests/WTF/CheckedPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/CompactPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/CompactRefPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/FixedVector.cpp:
* Tools/TestWebKitAPI/Tests/WTF/ListHashSet.cpp:
* Tools/TestWebKitAPI/Tests/WTF/NakedPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/PackedRefPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/RefPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/URL.cpp:
* Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:
* Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/cf/RetainPtr.cpp:
* Tools/TestWebKitAPI/Tests/WTF/cocoa/TypeCastsCocoa.mm:
* Tools/TestWebKitAPI/Tests/WTF/ns/RetainPtr.mm:
* Tools/TestWebKitAPI/Tests/WebKit/cocoa/WeakObjCPtr.mm:
- Ignore expected cplusplus.Move warnings using
  IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE.

Canonical link: https://commits.webkit.org/273460@main
  • Loading branch information
David Kilzer authored and ddkilzer committed Jan 25, 2024
1 parent 10fba3f commit 22343e6
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 22 deletions.
39 changes: 29 additions & 10 deletions Source/WTF/wtf/Compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@
/* COMPILER_QUIRK() - whether the compiler being used to build the project requires a given quirk. */
#define COMPILER_QUIRK(WTF_COMPILER_QUIRK) (defined WTF_COMPILER_QUIRK_##WTF_COMPILER_QUIRK && WTF_COMPILER_QUIRK_##WTF_COMPILER_QUIRK)

/* COMPILER_HAS_ATTRIBUTE() - whether the compiler supports a particular attribute. */
/* https://clang.llvm.org/docs/LanguageExtensions.html#has-attribute */
/* https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fattribute.html */
#ifdef __has_attribute
#define COMPILER_HAS_ATTRIBUTE(x) __has_attribute(x)
#else
#define COMPILER_HAS_ATTRIBUTE(x) 0
#endif

/* COMPILER_HAS_CLANG_BUILTIN() - whether the compiler supports a particular clang builtin. */
#ifdef __has_builtin
#define COMPILER_HAS_CLANG_BUILTIN(x) __has_builtin(x)
Expand All @@ -42,7 +51,7 @@
#endif

/* COMPILER_HAS_CLANG_FEATURE() - whether the compiler supports a particular language or library feature. */
/* http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension */
/* https://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension */
#ifdef __has_feature
#define COMPILER_HAS_CLANG_FEATURE(x) __has_feature(x)
#else
Expand Down Expand Up @@ -249,12 +258,9 @@

#elif !defined(FALLTHROUGH) && !defined(__cplusplus)

#if COMPILER(GCC_COMPATIBLE) && defined(__has_attribute)
// Break out this #if to satisy some versions Windows compilers.
#if __has_attribute(fallthrough)
#if COMPILER_HAS_ATTRIBUTE(fallthrough)
#define FALLTHROUGH __attribute__ ((fallthrough))
#endif
#endif

#endif // !defined(FALLTHROUGH) && defined(__cplusplus) && defined(__has_cpp_attribute)

Expand Down Expand Up @@ -302,8 +308,8 @@

/* NOT_TAIL_CALLED */

#if !defined(NOT_TAIL_CALLED) && defined(__has_attribute)
#if __has_attribute(not_tail_called)
#if !defined(NOT_TAIL_CALLED)
#if COMPILER_HAS_ATTRIBUTE(not_tail_called)
#define NOT_TAIL_CALLED __attribute__((not_tail_called))
#endif
#endif
Expand Down Expand Up @@ -539,7 +545,6 @@

#endif /* COMPILER(GCC) || COMPILER(CLANG) */


#if COMPILER(GCC)
#define IGNORE_GCC_WARNINGS_BEGIN(warning) IGNORE_WARNINGS_BEGIN_IMPL(GCC, warning)
#define IGNORE_GCC_WARNINGS_END IGNORE_WARNINGS_END_IMPL(GCC)
Expand All @@ -564,6 +569,17 @@
#define IGNORE_WARNINGS_END
#endif

/* IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_BEGIN() - whether the compiler supports suppressing static analysis warnings. */
/* https://clang.llvm.org/docs/AttributeReference.html#suppress */
#if COMPILER_HAS_ATTRIBUTE(suppress)
#define IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_ATTRIBUTE(warning, ...) [[clang::suppress]]
#define IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_BEGIN(warning, ...) [[clang::suppress]] {
#else
#define IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_ATTRIBUTE(warning, ...)
#define IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_BEGIN(warning, ...) {
#endif
#define IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_END }

#define ALLOW_DEPRECATED_DECLARATIONS_BEGIN IGNORE_WARNINGS_BEGIN("deprecated-declarations")
#define ALLOW_DEPRECATED_DECLARATIONS_END IGNORE_WARNINGS_END

Expand All @@ -585,6 +601,9 @@
#define ALLOW_NONLITERAL_FORMAT_BEGIN IGNORE_WARNINGS_BEGIN("format-nonliteral")
#define ALLOW_NONLITERAL_FORMAT_END IGNORE_WARNINGS_END

#define IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE \
IGNORE_CLANG_STATIC_ANALYZER_WARNINGS_ATTRIBUTE("cplusplus.Move")

#define IGNORE_RETURN_TYPE_WARNINGS_BEGIN IGNORE_WARNINGS_BEGIN("return-type")
#define IGNORE_RETURN_TYPE_WARNINGS_END IGNORE_WARNINGS_END

Expand All @@ -605,8 +624,8 @@

/* TLS_MODEL_INITIAL_EXEC */

#if !defined(TLS_MODEL_INITIAL_EXEC) && defined(__has_attribute)
#if __has_attribute(tls_model)
#if !defined(TLS_MODEL_INITIAL_EXEC)
#if COMPILER_HAS_ATTRIBUTE(tls_model)
#define TLS_MODEL_INITIAL_EXEC __attribute__((tls_model("initial-exec")))
#endif
#endif
Expand Down
2 changes: 2 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/CheckedPtr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ TEST(WTF_CheckedPtr, Basic)
EXPECT_EQ(checkedObject.ptrCount(), 2u);
EXPECT_EQ(ptr1.get(), &checkedObject);
EXPECT_EQ(ptr2.get(), &checkedObject);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(ptr3.get(), nullptr);
}
}
Expand Down Expand Up @@ -260,6 +261,7 @@ TEST(WTF_CheckedPtr, DerivedClass)
EXPECT_EQ(checkedObject.ptrCount(), 2u);
EXPECT_EQ(ptr1.get(), nullptr);
EXPECT_EQ(ptr2.get(), &checkedObject);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(ptr3.get(), nullptr);
EXPECT_EQ(ptr4.get(), &checkedObject);
}
Expand Down
5 changes: 5 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/CompactPtr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,15 @@ TEST(WTF_CompactPtr, Basic)
{
CompactPtr<AlignedRefLogger> p1 = &a;
CompactPtr<AlignedRefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}

{
CompactPtr<AlignedRefLogger> p1 = &a;
CompactPtr<AlignedRefLogger> p2(WTFMove(p1));
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand All @@ -95,6 +97,7 @@ TEST(WTF_CompactPtr, Basic)
{
CompactPtr<DerivedAlignedRefLogger> p1 = &a;
CompactPtr<AlignedRefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand Down Expand Up @@ -165,6 +168,7 @@ TEST(WTF_CompactPtr, Assignment)
EXPECT_EQ(&b, p2.get());
p1 = WTFMove(p2);
EXPECT_EQ(&b, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
}

Expand All @@ -185,6 +189,7 @@ TEST(WTF_CompactPtr, Assignment)
EXPECT_EQ(&c, p2.get());
p1 = WTFMove(p2);
EXPECT_EQ(&c, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
}

Expand Down
11 changes: 11 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/CompactRefPtr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ TEST(WTF_CompactRefPtr, Basic)
{
CompactRefPtr<AlignedRefLogger> p1 = &a;
CompactRefPtr<AlignedRefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand All @@ -85,6 +86,7 @@ TEST(WTF_CompactRefPtr, Basic)
{
CompactRefPtr<AlignedRefLogger> p1 = &a;
CompactRefPtr<AlignedRefLogger> p2(WTFMove(p1));
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand All @@ -101,6 +103,7 @@ TEST(WTF_CompactRefPtr, Basic)
{
CompactRefPtr<DerivedAlignedRefLogger> p1 = &a;
CompactRefPtr<AlignedRefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand Down Expand Up @@ -217,6 +220,7 @@ TEST(WTF_CompactRefPtr, Assignment)
log() << "| ";
p1 = WTFMove(p2);
EXPECT_EQ(&b, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
log() << "| ";
}
Expand Down Expand Up @@ -263,6 +267,7 @@ TEST(WTF_CompactRefPtr, Assignment)
log() << "| ";
p1 = WTFMove(p2);
EXPECT_EQ(&c, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
log() << "| ";
}
Expand Down Expand Up @@ -351,6 +356,7 @@ TEST(WTF_CompactRefPtr, Release)
{
CompactRefPtr<AlignedRefLogger> p1 = &a;
CompactRefPtr<AlignedRefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand All @@ -359,6 +365,7 @@ TEST(WTF_CompactRefPtr, Release)
{
CompactRefPtr<AlignedRefLogger> p1 = &a;
CompactRefPtr<AlignedRefLogger> p2(WTFMove(p1));
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand All @@ -367,6 +374,7 @@ TEST(WTF_CompactRefPtr, Release)
{
CompactRefPtr<DerivedAlignedRefLogger> p1 = &a;
CompactRefPtr<AlignedRefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p1.get());
EXPECT_EQ(&a, p2.get());
}
Expand All @@ -380,6 +388,7 @@ TEST(WTF_CompactRefPtr, Release)
log() << "| ";
p1 = WTFMove(p2);
EXPECT_EQ(&b, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
log() << "| ";
}
Expand All @@ -393,6 +402,7 @@ TEST(WTF_CompactRefPtr, Release)
log() << "| ";
p1 = WTFMove(p2);
EXPECT_EQ(&c, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
log() << "| ";
}
Expand Down Expand Up @@ -539,6 +549,7 @@ TEST(WTF_CompactRefPtr, AssignBeforeDeref)
a.slotToCheck = nullptr;
b.slotToCheck = nullptr;
EXPECT_EQ(&b, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(nullptr, p2.get());
log() << "| ";
}
Expand Down
4 changes: 4 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/FixedVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ TEST(WTF_FixedVector, Move)
vec1[2] = 2;

FixedVector<unsigned> vec2(WTFMove(vec1));
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE

This comment has been minimized.

Copy link
@cdumez

cdumez Feb 12, 2024

Contributor

Personally I find these super confusing. I have no idea what they apply to. I wish we'd use _BEGIN & _END to wrap the affected code, similarly to what we already do to suppress deprecation warnings.

Can we fix that?

This comment has been minimized.

Copy link
@cdumez

cdumez Feb 12, 2024

Contributor

I'm also not sure how I feel about indenting this with code, but this is minor in comparison.

EXPECT_EQ(0U, vec1.size());
EXPECT_EQ(3U, vec2.size());
for (unsigned i = 0; i < vec2.size(); ++i)
Expand All @@ -195,6 +196,7 @@ TEST(WTF_FixedVector, MoveAssign)

FixedVector<unsigned> vec2;
vec2 = WTFMove(vec1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(0U, vec1.size());
EXPECT_EQ(3U, vec2.size());
for (unsigned i = 0; i < vec2.size(); ++i)
Expand All @@ -206,6 +208,7 @@ TEST(WTF_FixedVector, MoveVector)
auto vec1 = Vector<MoveOnly>::from(MoveOnly(0), MoveOnly(1), MoveOnly(2), MoveOnly(3));
EXPECT_EQ(4U, vec1.size());
FixedVector<MoveOnly> vec2(WTFMove(vec1));
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(0U, vec1.size());
EXPECT_EQ(4U, vec2.size());
for (unsigned index = 0; index < vec2.size(); ++index)
Expand All @@ -219,6 +222,7 @@ TEST(WTF_FixedVector, MoveAssignVector)
auto vec1 = Vector<MoveOnly>::from(MoveOnly(0), MoveOnly(1), MoveOnly(2), MoveOnly(3));
EXPECT_EQ(4U, vec1.size());
vec2 = WTFMove(vec1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
EXPECT_EQ(0U, vec1.size());
}
EXPECT_EQ(4U, vec2.size());
Expand Down
2 changes: 2 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/ListHashSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ TEST(WTF_ListHashSet, MoveConstructor)
ASSERT_EQ(3, *iterator2);
++iterator2;

IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(0U, list.size());
ASSERT_TRUE(list.begin() == list.end());
list.add(4);
Expand Down Expand Up @@ -380,6 +381,7 @@ TEST(WTF_ListHashSet, MoveAssignment)
ASSERT_EQ(3, *iterator2);
++iterator2;

IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(0U, list.size());
ASSERT_TRUE(list.begin() == list.end());
list.add(4);
Expand Down
5 changes: 5 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/NakedPtr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,15 @@ TEST(WTF_NakedPtr, Basic)
{
NakedPtr<RefLogger> p1 = &a;
NakedPtr<RefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(&a, p1.get());
ASSERT_EQ(&a, p2.get());
}

{
NakedPtr<RefLogger> p1 = &a;
NakedPtr<RefLogger> p2(WTFMove(p1));
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(&a, p1.get());
ASSERT_EQ(&a, p2.get());
}
Expand All @@ -90,6 +92,7 @@ TEST(WTF_NakedPtr, Basic)
{
NakedPtr<DerivedRefLogger> p1 = &a;
NakedPtr<RefLogger> p2 = WTFMove(p1);
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(&a, p1.get());
ASSERT_EQ(&a, p2.get());
}
Expand Down Expand Up @@ -139,6 +142,7 @@ TEST(WTF_NakedPtr, Assignment)
ASSERT_EQ(&b, p2.get());
p1 = WTFMove(p2);
ASSERT_EQ(&b, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(&b, p2.get());
}

Expand Down Expand Up @@ -166,6 +170,7 @@ TEST(WTF_NakedPtr, Assignment)
ASSERT_EQ(&c, p2.get());
p1 = WTFMove(p2);
ASSERT_EQ(&c, p1.get());
IGNORE_CLANG_STATIC_ANALYZER_USE_AFTER_MOVE_ATTRIBUTE
ASSERT_EQ(&c, p2.get());
}

Expand Down

0 comments on commit 22343e6

Please sign in to comment.