[PATCH] D26632: [sanitizer] Track architecture and UUID of modules in LoadedModule

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 09:15:07 PST 2016


filcab added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common.h:686
+  ModuleArch arch() const { return arch_; }
+  const u8 *uuid() const { return &uuid_[0]; }
 
----------------
Why not just `return uuid_`?


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_mac.cc:139
+    return kModuleArchUnknown;
+  }
+}
----------------
How about an outer switch/case and then ternary ops/more ifs/another switch if needed?


================
Comment at: lib/sanitizer_common/sanitizer_procmaps_mac.cc:156
+  }
+}
+
----------------
Should we assert that we always find a UUID?


================
Comment at: lib/sanitizer_common/tests/sanitizer_procmaps_test.cc:66
+      } else if (SANITIZER_WORDSIZE == 64) {
+        EXPECT_TRUE(arch == kModuleArchX86_64 || arch == kModuleArchX86_64H);
+      }
----------------
Are the unit tests only run on X86(-64)? Never on ARM? Interesting.
Can you add a comment, please?


https://reviews.llvm.org/D26632





More information about the llvm-commits mailing list