Bug 193126 - AtomicStringImpl::isInAtomicStringTable used without check ASSERT_DISABLED
Summary: AtomicStringImpl::isInAtomicStringTable used without check ASSERT_DISABLED
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-03 15:32 PST by Zhifei Fang
Modified: 2019-01-04 09:53 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zhifei Fang 2019-01-03 15:32:29 PST
We declare isInAtomicStringTable like this:


#if !ASSERT_DISABLED
    WTF_EXPORT_STRING_API static bool isInAtomicStringTable(StringImpl*);
#endif


however we use it like this:


private:
    AtomicStringImpl() = delete;

    ALWAYS_INLINE static Ref<AtomicStringImpl> add(StringImpl& string)
    {
        if (string.isAtomic()) {
>           ASSERT_WITH_MESSAGE(!string.length() || isInAtomicStringTable(&string), "The atomic string comes from an other thread!");
                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            return static_cast<AtomicStringImpl&>(string);
        }
        return addSlowCase(string);
    }


if we define ASSERT_DISABLED this will cause a build failure
Comment 1 Zhifei Fang 2019-01-03 16:00:36 PST
root cause is 

ASSERT_WITH_MESSAGE not controlled by ASSERT_DISABLED
Comment 2 Alexey Proskuryakov 2019-01-04 09:53:06 PST
It doesn't look like there is a reason why ASSERT_DISABLED and ASSERT_MSG_DISABLED are separate. Perhaps we should unify them, or at least disable ASSERT_WITH_MESSAGE when ASSERT_DISABLED is set.