<div dir="ltr">Not going to block the CL over this, but consider using llvm::StringSwitch<> for improving the readability of this rather large if / else block.</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 28, 2016 at 6:14 PM Anna Zaks via Phabricator via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">zaks.anna added a comment.<br class="gmail_msg">
<br class="gmail_msg">
Here is another pass at this:<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:198<br class="gmail_msg">
   if (description == "heap-use-after-free") {<br class="gmail_msg">
     return "Use of deallocated memory detected";<br class="gmail_msg">
   } else if (description == "heap-buffer-overflow") {<br class="gmail_msg">
----------------<br class="gmail_msg">
Kuba would like to drop all of these "detected" that do not add anything and are just used as filler words in all of the description messages. This will make some of them into non-complete sentences, but keeping them short is more user friendly and gets the point across just fine.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:220<br class="gmail_msg">
+  } else if (description == "stack-overflow") {<br class="gmail_msg">
+    return "Stack overflow detected (recursion too deep)";<br class="gmail_msg">
+  } else if (description == "null-deref") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> kubabrecka wrote:<br class="gmail_msg">
> > filcab wrote:<br class="gmail_msg">
> > > Not necessarily recursion. There's also stack variables. I'd omit the stuff in parenthesis.<br class="gmail_msg">
> > Multiple times have I seen that people read "stack overflow" as "stack **buffer** overflow" and they spend a lot of time trying to find what buffer was actually overflown...  I'd like to somehow point that out.  Ideas?<br class="gmail_msg">
> Maybe instead of "recursion too deep" have "stack space exhausted" or something like that?<br class="gmail_msg">
> I've seen stack overflow errors on as few as 10 (maybe even fewer) stack frames (with big objects). ASan is also more likely to make this a problem. I think seeing "recursion too deep" but having only a dozen frames is probably confusing.<br class="gmail_msg">
> Not that "stack space exhausted" is much better, but I think it's less likely to be misleading.<br class="gmail_msg">
><br class="gmail_msg">
> And yes, please ask native speakers too, as I'm not one either. :-)<br class="gmail_msg">
><br class="gmail_msg">
We could just say "Stack space exhausted" with no parentheses.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:222<br class="gmail_msg">
+  } else if (description == "null-deref") {<br class="gmail_msg">
+    return "Null pointer dereference detected";<br class="gmail_msg">
+  } else if (description == "wild-jump") {<br class="gmail_msg">
----------------<br class="gmail_msg">
Drop "detected" or "Dereference of null pointer".<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:224<br class="gmail_msg">
+  } else if (description == "wild-jump") {<br class="gmail_msg">
+    return "Wild pointer jump detected";<br class="gmail_msg">
+  } else if (description == "wild-addr-write") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> Nit: "Wild jump" is probably better than "wild pointer jump", no?<br class="gmail_msg">
How about "Jump to a wild address". "Wild jump" sounds like a term, but I am not familiar with  it.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:226<br class="gmail_msg">
+  } else if (description == "wild-addr-write") {<br class="gmail_msg">
+    return "Write to a wild pointer detected";<br class="gmail_msg">
+  } else if (description == "wild-addr-read") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> Nit: I'd probably use "Write through wild pointer ..." (same for the others)<br class="gmail_msg">
"Write through", "Access through" but "Read from" (as per the English speaker on our team :)).<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:234<br class="gmail_msg">
+  } else if (description == "double-free") {<br class="gmail_msg">
+    return "Attempt to deallocate a freed object detected";<br class="gmail_msg">
+  } else if (description == "new-delete-type-mismatch") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> Nit: Phrasing seems off. I think "Double free detected" is easier to read and more explicit.<br class="gmail_msg">
Double free is a bit of a jargon, I think it's better to be explicit:<br class="gmail_msg">
"Deallocation of freed memory"<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:236<br class="gmail_msg">
+  } else if (description == "new-delete-type-mismatch") {<br class="gmail_msg">
+    return "Deallocation of a wrong size detected";<br class="gmail_msg">
+  } else if (description == "bad-free") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> Nit: Maybe "Deallocation size mismatch detected"? The user isn't deallocating a "size".<br class="gmail_msg">
Is this always about "new" and "delete"? If so, we could try to be more explicit about it "The size of deleted object does not match the size at allocation"<br class="gmail_msg">
<br class="gmail_msg">
"Deallocation size different than allocation size"?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:238<br class="gmail_msg">
+  } else if (description == "bad-free") {<br class="gmail_msg">
+    return "Attempt to free a non-allocated address detected";<br class="gmail_msg">
+  } else if (description == "alloc-dealloc-mismatch") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> s/address/memory region/?<br class="gmail_msg">
><br class="gmail_msg">
"Deallocation of non-allocated memory"<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:242<br class="gmail_msg">
+  } else if (description == "bad-malloc_usable_size") {<br class="gmail_msg">
+    return "Invalid call to malloc_usable_size detected";<br class="gmail_msg">
+  } else if (description == "bad-__sanitizer_get_allocated_size") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> Maybe "Call to malloc_usable_size with not owned pointer detected"?<br class="gmail_msg">
> Unsure if it's obvious why it's "invalid". Same for the one below.<br class="gmail_msg">
"Invalid argument to malloc_usable_size"?  And "Invalid argument to __sanitizer_get_allocated_size" below.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:246<br class="gmail_msg">
+  } else if (description == "param-overlap") {<br class="gmail_msg">
+    return "Overlapping memory ranges detected";<br class="gmail_msg">
+  } else if (description == "negative-size-param") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> I think this really needs additional detail. It doesn't seem very meaningful as a sentence.<br class="gmail_msg">
> Maybe "Overlapping memory ranges in function that doesn't allow them (detected?)" or something?<br class="gmail_msg">
> My suggestion can be improved too, for sure :-)<br class="gmail_msg">
How about this: "Call to function disallowing overlapping memory ranges"<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:248<br class="gmail_msg">
+  } else if (description == "negative-size-param") {<br class="gmail_msg">
+    return "Negative size memory access detected";<br class="gmail_msg">
+  } else if (description == "bad-__sanitizer_annotate_contiguous_container") {<br class="gmail_msg">
----------------<br class="gmail_msg">
Negative size used when accessing memory<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:250<br class="gmail_msg">
+  } else if (description == "bad-__sanitizer_annotate_contiguous_container") {<br class="gmail_msg">
+    return "Invalid call to __sanitizer_annotate_contiguous_container detected";<br class="gmail_msg">
+  } else if (description == "odr-violation") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> Maybe "Invalid parameters to call to __sanitizer_annotate_contiguous_container" is more explicit?<br class="gmail_msg">
> Maybe also a good solution for the similar cases above.<br class="gmail_msg">
"Invalid arguments to __sanitizer_annotate_contiguous_container"<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:252<br class="gmail_msg">
+  } else if (description == "odr-violation") {<br class="gmail_msg">
+    return "Initialization order problem detected";<br class="gmail_msg">
+  } else if (description == "invalid-pointer-pair") {<br class="gmail_msg">
----------------<br class="gmail_msg">
filcab wrote:<br class="gmail_msg">
> "One Definition Rule violation"<br class="gmail_msg">
> It's not an initialization order problem.<br class="gmail_msg">
Is it possible to use something similar to what clang uses, for example, "external variable %0 defined in multiple translation units" (from ./include//clang/Basic/DiagnosticASTKinds.td)?<br class="gmail_msg">
<br class="gmail_msg">
Ex: "Symbol defined in multiple translation units"<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:254<br class="gmail_msg">
+  } else if (description == "invalid-pointer-pair") {<br class="gmail_msg">
+    return "Invalid pointer pair in arithmetic operation detected";<br class="gmail_msg">
   }<br class="gmail_msg">
----------------<br class="gmail_msg">
Comparison or arithmetic on pointers from different memory regions<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
Repository:<br class="gmail_msg">
  rL LLVM<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D27017" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27017</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
lldb-commits mailing list<br class="gmail_msg">
<a href="mailto:lldb-commits@lists.llvm.org" class="gmail_msg" target="_blank">lldb-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br class="gmail_msg">
</blockquote></div>