[TSan] Use switches when dealing with enums

[yln]

Summary

Small refactoring: replace some if-else cascades with switches so that the compiler warns us about missing cases.
Maybe found a small bug?

[yln]

lib/tsan/rtl/tsan_suppressions.cc

96 ↗

(On Diff #180149)

Should this be kSuppressionSignal?

[delcypher]

lib/tsan/rtl/tsan_debugging.cc

41 ↗

(On Diff #180149)

@yln I'd feel slightly happier by having UNREACHABLE("Unhandled case"); after the switch block (i.e. don't add a default case) so that if we do fall through (because someone missed a compiler warning, or perhaps they have warnings turned off) we stop execution rather than having undefined behaviour (reach end of function but no return statement).

This applies to all the switch blocks you've added.

I won't insist on this but if you think this a reasonable change that doesn't clutter things too much then I think you should do it.

[yln]

Add UNREACHABLE("missing case");

[yln]

lib/tsan/rtl/tsan_debugging.cc

41 ↗

(On Diff #180149)

Good point! Thanks for the review, Dan.

lib/tsan/rtl/tsan_suppressions.cc

96 ↗

(On Diff #180149)

@dvyukov @kubamracek: can you please confirm that this is intentional and not an oversight?

compiler-rt/trunk/lib/tsan/rtl/tsan_debugging.cc

 #include "tsan_report.h"
 #include "tsan_rtl.h"
 
 #include "sanitizer_common/sanitizer_stackdepot.h"
 
 using namespace __tsan;
 
 static const char *ReportTypeDescription(ReportType typ) {
-  if (typ == ReportTypeRace) return "data-race";
-  if (typ == ReportTypeVptrRace) return "data-race-vptr";
-  if (typ == ReportTypeUseAfterFree) return "heap-use-after-free";
-  if (typ == ReportTypeVptrUseAfterFree) return "heap-use-after-free-vptr";
-  if (typ == ReportTypeExternalRace) return "external-race";
-  if (typ == ReportTypeThreadLeak) return "thread-leak";
-  if (typ == ReportTypeMutexDestroyLocked) return "locked-mutex-destroy";
-  if (typ == ReportTypeMutexDoubleLock) return "mutex-double-lock";
-  if (typ == ReportTypeMutexInvalidAccess) return "mutex-invalid-access";
-  if (typ == ReportTypeMutexBadUnlock) return "mutex-bad-unlock";
-  if (typ == ReportTypeMutexBadReadLock) return "mutex-bad-read-lock";
-  if (typ == ReportTypeMutexBadReadUnlock) return "mutex-bad-read-unlock";
-  if (typ == ReportTypeSignalUnsafe) return "signal-unsafe-call";
-  if (typ == ReportTypeErrnoInSignal) return "errno-in-signal-handler";
-  if (typ == ReportTypeDeadlock) return "lock-order-inversion";
-  return "";
+  switch (typ) {
+    case ReportTypeRace: return "data-race";
+    case ReportTypeVptrRace: return "data-race-vptr";
+    case ReportTypeUseAfterFree: return "heap-use-after-free";
+    case ReportTypeVptrUseAfterFree: return "heap-use-after-free-vptr";
+    case ReportTypeExternalRace: return "external-race";
+    case ReportTypeThreadLeak: return "thread-leak";
+    case ReportTypeMutexDestroyLocked: return "locked-mutex-destroy";
+    case ReportTypeMutexDoubleLock: return "mutex-double-lock";
+    case ReportTypeMutexInvalidAccess: return "mutex-invalid-access";
+    case ReportTypeMutexBadUnlock: return "mutex-bad-unlock";
+    case ReportTypeMutexBadReadLock: return "mutex-bad-read-lock";
+    case ReportTypeMutexBadReadUnlock: return "mutex-bad-read-unlock";
+    case ReportTypeSignalUnsafe: return "signal-unsafe-call";
+    case ReportTypeErrnoInSignal: return "errno-in-signal-handler";
+    case ReportTypeDeadlock: return "lock-order-inversion";
+    // No default case so compiler warns us if we miss one
+  }
+  UNREACHABLE("missing case");
 }
 
 static const char *ReportLocationTypeDescription(ReportLocationType typ) {
-  if (typ == ReportLocationGlobal) return "global";
-  if (typ == ReportLocationHeap) return "heap";
-  if (typ == ReportLocationStack) return "stack";
-  if (typ == ReportLocationTLS) return "tls";
-  if (typ == ReportLocationFD) return "fd";
-  return "";
+  switch (typ) {
+    case ReportLocationGlobal: return "global";
+    case ReportLocationHeap: return "heap";
+    case ReportLocationStack: return "stack";
+    case ReportLocationTLS: return "tls";
+    case ReportLocationFD: return "fd";
+    // No default case so compiler warns us if we miss one
+  }
+  UNREACHABLE("missing case");
 }
 
 static void CopyTrace(SymbolizedStack *first_frame, void **trace,
                       uptr trace_size) {
   uptr i = 0;
   for (SymbolizedStack *frame = first_frame; frame != nullptr;
        frame = frame->next) {
     trace[i++] = (void *)frame->info.address;

compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc

 const char *thread_name(char *buf, int tid) {
   if (tid == 0)
     return "main thread";
   internal_snprintf(buf, kThreadBufSize, "thread T%d", tid);
   return buf;
 }
 
 static const char *ReportTypeString(ReportType typ, uptr tag) {
-  if (typ == ReportTypeRace)
+  switch (typ) {
+    case ReportTypeRace:
       return "data race";
-  if (typ == ReportTypeVptrRace)
+    case ReportTypeVptrRace:
       return "data race on vptr (ctor/dtor vs virtual call)";
-  if (typ == ReportTypeUseAfterFree)
+    case ReportTypeUseAfterFree:
       return "heap-use-after-free";
-  if (typ == ReportTypeVptrUseAfterFree)
+    case ReportTypeVptrUseAfterFree:
       return "heap-use-after-free (virtual call vs free)";
-  if (typ == ReportTypeExternalRace) {
+    case ReportTypeExternalRace: {
       const char *str = GetReportHeaderFromTag(tag);
       return str ? str : "race on external object";
     }
-  if (typ == ReportTypeThreadLeak)
+    case ReportTypeThreadLeak:
       return "thread leak";
-  if (typ == ReportTypeMutexDestroyLocked)
+    case ReportTypeMutexDestroyLocked:
       return "destroy of a locked mutex";
-  if (typ == ReportTypeMutexDoubleLock)
+    case ReportTypeMutexDoubleLock:
       return "double lock of a mutex";
-  if (typ == ReportTypeMutexInvalidAccess)
+    case ReportTypeMutexInvalidAccess:
       return "use of an invalid mutex (e.g. uninitialized or destroyed)";
-  if (typ == ReportTypeMutexBadUnlock)
+    case ReportTypeMutexBadUnlock:
       return "unlock of an unlocked mutex (or by a wrong thread)";
-  if (typ == ReportTypeMutexBadReadLock)
+    case ReportTypeMutexBadReadLock:
       return "read lock of a write locked mutex";
-  if (typ == ReportTypeMutexBadReadUnlock)
+    case ReportTypeMutexBadReadUnlock:
       return "read unlock of a write locked mutex";
-  if (typ == ReportTypeSignalUnsafe)
+    case ReportTypeSignalUnsafe:
       return "signal-unsafe call inside of a signal";
-  if (typ == ReportTypeErrnoInSignal)
+    case ReportTypeErrnoInSignal:
       return "signal handler spoils errno";
-  if (typ == ReportTypeDeadlock)
+    case ReportTypeDeadlock:
       return "lock-order-inversion (potential deadlock)";
-  return "";
+    // No default case so compiler warns us if we miss one
+  }
+  UNREACHABLE("missing case");
 }
 
 #if SANITIZER_MAC
 static const char *const kInterposedFunctionPrefix = "wrap_";
 #else
 static const char *const kInterposedFunctionPrefix = "__interceptor_";
 #endif
 

compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc

 }
 
 SuppressionContext *Suppressions() {
   CHECK(suppression_ctx);
   return suppression_ctx;
 }
 
 static const char *conv(ReportType typ) {
-  if (typ == ReportTypeRace)
+  switch (typ) {
+    case ReportTypeRace:
+    case ReportTypeVptrRace:
+    case ReportTypeUseAfterFree:
+    case ReportTypeVptrUseAfterFree:
+    case ReportTypeExternalRace:
       return kSuppressionRace;
-  else if (typ == ReportTypeVptrRace)
-    return kSuppressionRace;
-  else if (typ == ReportTypeUseAfterFree)
-    return kSuppressionRace;
-  else if (typ == ReportTypeVptrUseAfterFree)
-    return kSuppressionRace;
-  else if (typ == ReportTypeExternalRace)
-    return kSuppressionRace;
-  else if (typ == ReportTypeThreadLeak)
+    case ReportTypeThreadLeak:
       return kSuppressionThread;
-  else if (typ == ReportTypeMutexDestroyLocked)
-    return kSuppressionMutex;
-  else if (typ == ReportTypeMutexDoubleLock)
+    case ReportTypeMutexDestroyLocked:
+    case ReportTypeMutexDoubleLock:
+    case ReportTypeMutexInvalidAccess:
+    case ReportTypeMutexBadUnlock:
+    case ReportTypeMutexBadReadLock:
+    case ReportTypeMutexBadReadUnlock:
       return kSuppressionMutex;
-  else if (typ == ReportTypeMutexInvalidAccess)
-    return kSuppressionMutex;
-  else if (typ == ReportTypeMutexBadUnlock)
-    return kSuppressionMutex;
-  else if (typ == ReportTypeMutexBadReadLock)
-    return kSuppressionMutex;
-  else if (typ == ReportTypeMutexBadReadUnlock)
-    return kSuppressionMutex;
-  else if (typ == ReportTypeSignalUnsafe)
+    case ReportTypeSignalUnsafe:
+    case ReportTypeErrnoInSignal:
       return kSuppressionSignal;
-  else if (typ == ReportTypeErrnoInSignal)
-    return kSuppressionNone;
-  else if (typ == ReportTypeDeadlock)
+    case ReportTypeDeadlock:
       return kSuppressionDeadlock;
-  Printf("ThreadSanitizer: unknown report type %d\n", typ);
-  Die();
+    // No default case so compiler warns us if we miss one
+  }
+  UNREACHABLE("missing case");
 }
 
 static uptr IsSuppressed(const char *stype, const AddressInfo &info,
     Suppression **sp) {
   if (suppression_ctx->Match(info.function, stype, sp) ||
       suppression_ctx->Match(info.file, stype, sp) ||
       suppression_ctx->Match(info.module, stype, sp)) {
     VPrintf(2, "ThreadSanitizer: matched suppression '%s'\n", (*sp)->templ);