Undef _TIME_BITS along with _FILE_OFFSET_BITS in sanitizers

[raj.khem]

Summary

On 32bit systems using 64bit time_t build fails because
_FILE_OFFSET_BITS is undefined here but _TIME_BITS is still set to 64

Fixes
In file included from compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp:17:
In file included from compiler-rt/lib/sanitizer_common/sanitizer_platform.h:25:
In file included from /usr/include/features.h:393:
/usr/include/features-time64.h:26:5: error: "_TIME_BITS=64 is allowed only with _FILE_OFFSET_BITS=64"

^

1 error generated.

[MaskRay]

Reasonable to me, but add @ro to check.

[ro]

Please state what target the problem occurs on and how/where the patch was tested. _TIME_BITS doesn't exist on Solaris 11.4 and, AFAICS, neither on Illumos.

If the patch goes in, please add a comment why that #undef is needed.

That said, I wonder why sanitizer_platform_limits_posix.cpp doesn't have the same issue. It seems weird to me to work around a non-Solaris issue in a Solaris-specific file. ISTM that <features.h> is included in sanitizer_platform.h only to get a definition of __GLIBC__. I'd find it way more logical to handle the issue there, rather than in sanitizer_procmaps_solaris.cpp.

[raj.khem]

In D144514#4142654, @ro wrote:

Please state what target the problem occurs on and how/where the patch was tested. _TIME_BITS doesn't exist on Solaris 11.4 and, AFAICS, neither on Illumos.

If the patch goes in, please add a comment why that #undef is needed.

That said, I wonder why sanitizer_platform_limits_posix.cpp doesn't have the same issue. It seems weird to me to work around a non-Solaris issue in a Solaris-specific file. ISTM that <features.h> is included in sanitizer_platform.h only to get a definition of __GLIBC__. I'd find it way more logical to handle the issue there, rather than in sanitizer_procmaps_solaris.cpp.

I have moved the undef to sanitizer_platform.h, sadly it has to be included before feature.h, let me know if you have any feedback on this new patch

[ro]

• You still haven't mentioned which target is affected.
• There needs to be a comment explaining why _TIME_BITS is #undefed.
• Please pass the patch through clang-format-diff.py: there's a TAB before !defined which should just be a blank.

[raj.khem]

In D144514#4144195, @ro wrote:

• You still haven't mentioned which target is affected.
• There needs to be a comment explaining why _TIME_BITS is #undefed.
• Please pass the patch through clang-format-diff.py: there's a TAB before !defined which should just be a blank.

Thanks for comments. I have updated the changeset to address this feedback

[MaskRay]

compiler-rt/lib/sanitizer_common/sanitizer_platform.h

30 ↗

(On Diff #499563)

compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp

 //===-- sanitizer_procmaps_solaris.cpp ------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Information about the process mappings (Solaris-specific parts).
 //===----------------------------------------------------------------------===//
 
 // Before Solaris 11.4, <procfs.h> doesn't work in a largefile environment.
 #undef _FILE_OFFSET_BITS
+#undef _TIME_BITS
 #include "sanitizer_platform.h"
 #if SANITIZER_SOLARIS
 #  include <fcntl.h>
 #  include <limits.h>
 #  include <procfs.h>
 
 #  include "sanitizer_common.h"
 #  include "sanitizer_procmaps.h"