[scudo][NFC] Suppress warnings for missing-noreturn, conditional-uninitialized, zero-length-array

[ddcc]

@vitalybuka ping

[vitalybuka]

compiler-rt/lib/scudo/standalone/memtag.h

44

can you avoid pragmas with NORETURN?

compiler-rt/lib/scudo/standalone/secondary.h

247–274 ↗

(On Diff #415977)

248 ↗

(On Diff #415977)

Please don't mix different warning types in a single patch?

400 ↗

(On Diff #415977)

Can you avoid this by extractioing Quarantine with template specialization

class <size_t Size>
class Quarantine {
  doStuff1() {blocks...}
  doStuff2() {blocks...}
private
  CachedBlock blocks[Size];
};

template<> class Quarantine<0> {
  doStuff1() {}
  doStuff2() {}
}

[ddcc]

compiler-rt/lib/scudo/standalone/memtag.h

44

Sorry, I don't understand the question. The UNREACHABLE macro calls the die() function that is labeled with NORETURN, which causes the compiler to warn about functions that don't return but that aren't labeled NORETURN. I can remove NORETURN from that function, but that would also require changing its' callers as well, such as dieOnMapUnmapError.

compiler-rt/lib/scudo/standalone/secondary.h

247–274 ↗

(On Diff #415977)

The issue is on HeaderPos; for some reason the compiler thinks it may be uninitialized. I can just initialize that to zero instead.

400 ↗

(On Diff #415977)

I'll split this off into another patch.

[ddcc]

Initialize HeaderPos, drop Quarantine changes

[vitalybuka]

zero-length-array in description is not needed

compiler-rt/lib/scudo/standalone/memtag.h

44

I am asking about:
NORETURN inline uptr archMemoryTagGranuleSize()

If this does not suppress the warning then just
return 0;

Which compiler do you use?
Can you paste warning content with context?

[ddcc]

Add noreturn attribute

[ddcc]

compiler-rt/lib/scudo/standalone/memtag.h

44

Oh sorry I misunderstood. Yeah adding noreturn avoids most of the warnings, except for the one on setRandomTag. This is because that warning only gets emitted while compiling memtag_test.cpp, which does not include combined.h, which has the only non-fatal uses of setRandomTag. But it's not correct to declare it noreturn in general, because the other uses are not fatal.

include/internal/scudo/memtag.h:305:62: error: function 'setRandomTag' could be declared with attribute 'noreturn'
      [-Werror,-Wmissing-noreturn]
                         uptr *TaggedBegin, uptr *TaggedEnd) {
                                                             ^

include/internal/scudo/memtag.h:308:1: error: function declared 'noreturn' should not return [-Werror,-Winvalid-noreturn]
}

[vitalybuka]

compiler-rt/lib/scudo/standalone/memtag.h

44

I don't like pragma there but NORETURN setRandomTag is incorrect.

[vitalybuka]

BTW, I suspect your fixes are going to regress with the time.
Would you like to update scudo_standalone cmake files so buildbots check these warnings?

[ddcc]

In D121853#3414960, @vitalybuka wrote:

BTW, I suspect your fixes are going to regress with the time.
Would you like to update scudo_standalone cmake files so buildbots check these warnings?

Sure, I'll handle that in a follow-up. I'm not familiar with how the current build; are scudo/CMakeLists.txt and scudo/standalone/CMakeLists.txt separate projects? Are there instructions somewhere for just building standalone scudo instead of compiler-rt + scudo?

[vitalybuka]

In D121853#3415113, @ddcc wrote:

In D121853#3414960, @vitalybuka wrote:

BTW, I suspect your fixes are going to regress with the time.
Would you like to update scudo_standalone cmake files so buildbots check these warnings?

Sure, I'll handle that in a follow-up. I'm not familiar with how the current build; are scudo/CMakeLists.txt and scudo/standalone/CMakeLists.txt separate projects? Are there instructions somewhere for just building standalone scudo instead of compiler-rt + scudo?

standalone is replacement of scudo which uses no stuff from compiler-rt/lib/sanitizer_common/
scudo (not scudo/standalone) should be deleted and @hctim has some pending patch, so ignore that one.

bots should do: ninja check-scudo_standalone
e.g. here https://lab.llvm.org/buildbot/#/builders/37/builds/11999

[abrachet]

compiler-rt/lib/scudo/standalone/memtag.h

304

These need to be wrapped in #if __clang__. This doesn't build with gcc + -Werror=unknown-pragmas

[mcgrathr]

compiler-rt/lib/scudo/standalone/memtag.h

304

For warning switches that GCC also has, you should instead be using #pragma GCC diagnostic ....
Clang's feature is the same as GCC's, and Clang also supports the GCC syntax (which predates the Clang-specific pragma AFAIK). -Wmissing-noreturn is a warning switch both compilers have and that works the same in both.

[ddcc]

Reverting for now, looks like there's some other build failures.

compiler-rt/lib/scudo/standalone/memtag.h

 inline uptr untagPointer(uptr Ptr) { return Ptr & ((1ULL << 56) - 1); }
 
 inline uint8_t extractTag(uptr Ptr) { return (Ptr >> 56) & 0xf; }
 
 #else
 
 inline constexpr bool archSupportsMemoryTagging() { return false; }
 
-inline uptr archMemoryTagGranuleSize() {
+inline NORETURN uptr archMemoryTagGranuleSize() {
   UNREACHABLE("memory tagging not supported");
 }
 
-inline uptr untagPointer(uptr Ptr) {
+inline NORETURN uptr untagPointer(uptr Ptr) {
   (void)Ptr;
   UNREACHABLE("memory tagging not supported");
 }
 
-inline uint8_t extractTag(uptr Ptr) {
+inline NORETURN uint8_t extractTag(uptr Ptr) {
   (void)Ptr;
   UNREACHABLE("memory tagging not supported");
 }
 
 #endif
 
 #if __clang_major__ >= 12 && defined(__aarch64__)
 
prctl(PR_SET_TAGGED_ADDR_CTRL,
         PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | (0xfffe << PR_MTE_TAG_SHIFT),
         0, 0, 0);
 }
 
 #else // !SCUDO_LINUX
 
 inline bool systemSupportsMemoryTagging() { return false; }
 
-inline bool systemDetectsMemoryTagFaultsTestOnly() {
+inline NORETURN bool systemDetectsMemoryTagFaultsTestOnly() {
   UNREACHABLE("memory tagging not supported");
 }
 
-inline void enableSystemMemoryTaggingTestOnly() {
+inline NORETURN void enableSystemMemoryTaggingTestOnly() {
   UNREACHABLE("memory tagging not supported");
 }
 
 #endif // SCUDO_LINUX
 
 class ScopedDisableMemoryTagChecks {
   uptr PrevTCO;
 
__asm__ __volatile__(
       : "+r"(TaggedPtr)
       :
       : "memory");
   return TaggedPtr;
 }
 
 #else
 
-inline bool systemSupportsMemoryTagging() {
+inline NORETURN bool systemSupportsMemoryTagging() {
   UNREACHABLE("memory tagging not supported");
 }
 
-inline bool systemDetectsMemoryTagFaultsTestOnly() {
+inline NORETURN bool systemDetectsMemoryTagFaultsTestOnly() {
   UNREACHABLE("memory tagging not supported");
 }
 
-inline void enableSystemMemoryTaggingTestOnly() {
+inline NORETURN void enableSystemMemoryTaggingTestOnly() {
   UNREACHABLE("memory tagging not supported");
 }
 
 struct ScopedDisableMemoryTagChecks {
   ScopedDisableMemoryTagChecks() {}
 };
 
-inline uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) {
+inline NORETURN uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) {
   (void)Ptr;
   (void)ExcludeMask;
   UNREACHABLE("memory tagging not supported");
 }
 
-inline uptr addFixedTag(uptr Ptr, uptr Tag) {
+inline NORETURN uptr addFixedTag(uptr Ptr, uptr Tag) {
   (void)Ptr;
   (void)Tag;
   UNREACHABLE("memory tagging not supported");
 }
 
-inline uptr storeTags(uptr Begin, uptr End) {
+inline NORETURN uptr storeTags(uptr Begin, uptr End) {
   (void)Begin;
   (void)End;
   UNREACHABLE("memory tagging not supported");
 }
 
-inline void storeTag(uptr Ptr) {
+inline NORETURN void storeTag(uptr Ptr) {
   (void)Ptr;
   UNREACHABLE("memory tagging not supported");
 }
 
-inline uptr loadTag(uptr Ptr) {
+inline NORETURN uptr loadTag(uptr Ptr) {
   (void)Ptr;
   UNREACHABLE("memory tagging not supported");
 }
 
 #endif
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wmissing-noreturn"
 inline void setRandomTag(void *Ptr, uptr Size, uptr ExcludeMask,
                          uptr *TaggedBegin, uptr *TaggedEnd) {
   *TaggedBegin = selectRandomTag(reinterpret_cast<uptr>(Ptr), ExcludeMask);
   *TaggedEnd = storeTags(*TaggedBegin, *TaggedBegin + Size);
 }
+#pragma clang diagnostic pop
 
 inline void *untagPointer(void *Ptr) {
   return reinterpret_cast<void *>(untagPointer(reinterpret_cast<uptr>(Ptr)));
 }
 
 inline void *loadTag(void *Ptr) {
   return reinterpret_cast<void *>(loadTag(reinterpret_cast<uptr>(Ptr)));
 }