[PATCH] D24132: Remove the old AddressDescription struct

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 7 07:10:59 PDT 2016


filcab abandoned this revision.
filcab added a comment.

This revision got merged with https://reviews.llvm.org/D24131.


================
Comment at: lib/asan/asan_debugging.cc:85
@@ +84,3 @@
+  uptr region_size = 0;
+  const char *region_kind;
+  switch (descr.kind) {
----------------
vitalybuka wrote:
> Could you please add:
> const char *region_kind = nullptr;
> name[0] = 0;
> 
> if any bug in switches below, it would be easiest to debug consistent values
I usually avoid initializing when we cover all cases, as it makes it easier for msan to catch the uninitialized use.
But this sanitizer won't be built with MSan, so that defensive coding is a good thing to do. Thanks.

================
Comment at: lib/asan/asan_debugging.cc:86
@@ +85,3 @@
+  const char *region_kind;
+  switch (descr.kind) {
+    case kAddressKindWild:
----------------
vitalybuka wrote:
> what is going to happen if descr.kind is not one of your cases?
> 
> if it's impossible, CHECK in default: would be nice.
> 
If we add the default case, we get a clang warning, though. I can add a check that we have one of those, before the switch, but it will end up ugly:

```
CHECK(descr.kind == kAddressKindWild || descr.kind == kAddressKindShadow || ...);
```

Should we also fix places like `PrintHeapChunkAccess` in asan_descriptions.cc, which does the same thing (not have a default case)?


https://reviews.llvm.org/D24132





More information about the llvm-commits mailing list