filcab added a comment.
I have some minor fixes I'd like to see.
If it's prefixed by "Nit: " it's a really minor one and I'm ok with it as is if
that's what you prefer.
Thank you,
Filipe
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:220
+ } else if (description == "stack-overflow") {
+ return "Stack overflow detected (recursion too deep)";
+ } else if (description == "null-deref") {
----------------
Not necessarily recursion. There's also stack variables. I'd omit the stuff in
parenthesis.
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:224
+ } else if (description == "wild-jump") {
+ return "Wild pointer jump detected";
+ } else if (description == "wild-addr-write") {
----------------
Nit: "Wild jump" is probably better than "wild pointer jump", no?
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:226
+ } else if (description == "wild-addr-write") {
+ return "Write to a wild pointer detected";
+ } else if (description == "wild-addr-read") {
----------------
Nit: I'd probably use "Write through wild pointer ..." (same for the others)
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:234
+ } else if (description == "double-free") {
+ return "Attempt to deallocate a freed object detected";
+ } else if (description == "new-delete-type-mismatch") {
----------------
Nit: Phrasing seems off. I think "Double free detected" is easier to read and
more explicit.
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:236
+ } else if (description == "new-delete-type-mismatch") {
+ return "Deallocation of a wrong size detected";
+ } else if (description == "bad-free") {
----------------
Nit: Maybe "Deallocation size mismatch detected"? The user isn't deallocating a
"size".
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:238
+ } else if (description == "bad-free") {
+ return "Attempt to free a non-allocated address detected";
+ } else if (description == "alloc-dealloc-mismatch") {
----------------
s/address/memory region/?
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:242
+ } else if (description == "bad-malloc_usable_size") {
+ return "Invalid call to malloc_usable_size detected";
+ } else if (description == "bad-__sanitizer_get_allocated_size") {
----------------
Maybe "Call to malloc_usable_size with not owned pointer detected"?
Unsure if it's obvious why it's "invalid". Same for the one below.
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:246
+ } else if (description == "param-overlap") {
+ return "Overlapping memory ranges detected";
+ } else if (description == "negative-size-param") {
----------------
I think this really needs additional detail. It doesn't seem very meaningful as
a sentence.
Maybe "Overlapping memory ranges in function that doesn't allow them
(detected?)" or something?
My suggestion can be improved too, for sure :-)
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:250
+ } else if (description == "bad-__sanitizer_annotate_contiguous_container") {
+ return "Invalid call to __sanitizer_annotate_contiguous_container
detected";
+ } else if (description == "odr-violation") {
----------------
Maybe "Invalid parameters to call to __sanitizer_annotate_contiguous_container"
is more explicit?
Maybe also a good solution for the similar cases above.
================
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:252
+ } else if (description == "odr-violation") {
+ return "Initialization order problem detected";
+ } else if (description == "invalid-pointer-pair") {
----------------
"One Definition Rule violation"
It's not an initialization order problem.
Repository:
rL LLVM
https://reviews.llvm.org/D27017
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits