[llvm-dev] How to resolve conflicts between sanitizer_common and system headers

[Dmitry Vyukov via llvm-dev]

On Fri, Jul 1, 2016 at 10:03 PM, Anna Zaks <ganna at apple.com> wrote:
>
>> On Jul 1, 2016, at 12:10 PM, Dmitry Vyukov <dvyukov at google.com> wrote:
>>
>> On Fri, Jul 1, 2016 at 8:53 PM, Anna Zaks <ganna at apple.com> wrote:
>>> Hi Sanitizer Runtime Developers,
>>>
>>> We recently ran into a problem building clang because some of the
>>> definitions in sanitizer_common conflicted with system definitions and later
>>> another system header was trying to use the system definition:
>>>
>>> .../usr/include/libkern/OSAtomicDeprecated.h:756:17: error: reference to
>>> 'memory_order_relaxed' is ambiguous
>>> __theAmount, memory_order_relaxed) + __theAmount);
>>> ^
>>> .../usr/bin/../include/c++/v1/atomic:548:5: note: candidate found by name
>>> lookup is 'std::__1::memory_order::memory_order_relaxed'
>>> memory_order_relaxed, memory_order_consume, memory_order_acquire,
>>> ^
>>> ../src/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_atomic.h:22:3:
>>> note: candidate found by name lookup is
>>> '__sanitizer::memory_order::memory_order_relaxed'
>>> memory_order_relaxed = 1 << 0,
>>> ^
>>>
>>>
>>> The problem is due to the combination of the following:
>>> 1. The runtime includes the system headers after the project headers (as
>>> per LLVM coding guidelines).
>>> 2. lib/sanitizer_common/sanitizer_internal_defs.h pollutes the namespace of
>>> everything defined after it, which is all/most of the sanitizer .h and .cc
>>> files and the included system headers with:
>>>    using namespace __sanitizer;  // NOLINT
>>> 3. These are the definitions that conflict in this particular case, but
>>> this problem could reoccur in the future with other symbols as well:
>>>
>>> enum memory_order {
>>>  memory_order_relaxed = 1 << 0,
>>>  memory_order_consume = 1 << 1,
>>>  memory_order_acquire = 1 << 2,
>>>  memory_order_release = 1 << 3,
>>>  memory_order_acq_rel = 1 << 4,
>>>  memory_order_seq_cst = 1 << 5
>>> };
>>>
>>>
>>> We currently have a workaround (in the system header) that makes this
>>> non-blocking, but it would be good to cleanly address this problem. Removing
>>> the "using namespace" from the header seems like the cleanest solution.
>>> WDYT?
>>>
>>> Thanks,
>>> Anna.
>>
>>
>> Hi Anna,
>>
>> What does OSAtomicDeprecated.h do? Does it also pull
>> memory_order_relaxed into global namespace?
>
> The header used to use std namespace locally before accessing those symbols, which caused the error when building clang:
>
> #define _OSATOMIC_USING_NAMESPACE_STD() using namespace std
> ...
> {
>         _OSATOMIC_USING_NAMESPACE_STD();
>         return (atomic_fetch_add_explicit((volatile _OSAtomic_int32_t*) __theValue,
>                         __theAmount, memory_order_relaxed) + __theAmount);
> }
>
> They worked around the problem by qualifying the symbol with "std::":
> #define OSATOMIC_STD(_a) std::_a
> ...
> {
>         return (OSATOMIC_STD(atomic_fetch_add_explicit)(
>                    (volatile _OSAtomic_int32_t*) __theValue, __theAmount,
>                    OSATOMIC_STD(memory_order_relaxed)) + __theAmount);
> }
>
> There are quite a few of these conflicts so it made the system header uglier. On the other hand, we do have a workaround.
>
> It would be great to have a general solution that would prevent this issue from happening in the future.
>
> + Bob, who reported the problem.
>
> Anna.
>>
>> For this particular problem we could just rename sanitizer internal constants.
>>
>> Maybe doing something like "namespace __tsan { using namespace
>> __sanitizer_common; }" will help to resolve it in general case. But I
>> am not sure, need to know what system headers do.


For such case we indeed can change
using namespace __sanitizer_common;
to:
namespace __tsan { using namespace __sanitizer_common; }


Here is my litmus test. Fails without -DFIX, compiles successfully with -DFIX.

namespace sanitizer {
        const int FOO = 1;
}
#ifdef FIX
namespace tsan { using namespace sanitizer; }
#else
using namespace sanitizer;
#endif

namespace cxx {
        const int FOO = 1;
}

namespace tsan {
        const int BAR = FOO;
}

int main() {
        using namespace cxx;
        const int BAR = FOO;
        return 0;
}