I like the version without "function" in it; you might be returning from a
method.
On Fri, Dec 2, 2016 at 12:15 PM, Filipe Cabecinhas <
filcab+llvm.phabrica...@gmail.com> wrote:
> .Case("stack-use-after-return", "Use of returned stack memory")
>
>
> Maybe "Use of stack memory after (function)
This commit is causing tests to fail in Darwin:
TestReportData.AsanTestReportDataCase
TestMemoryHistory.AsanTestCase
http://lab.llvm.org:8080/green/job/lldb_build_test/22768/
http://lab.llvm.org:8080/green/job/lldb_build_test/22769/
FAIL: test_dwarf (TestReportData.AsanTestReportDataCase)
kubabrecka added a comment.
Thanks for reviewing this!
Repository:
rL LLVM
https://reviews.llvm.org/D27017
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288535: Support more report types in
AddressSanitizerRuntime.cpp, re-word existing ones. (authored by kuba.brecka).
Changed prior to commit:
https://reviews.llvm.org/D27017?vs=79611=80123#toc
.Case("stack-use-after-return", "Use of returned stack memory")
Maybe "Use of stack memory after (function) return"? (i couldn't decide
whether to include "function". Either delete it or delete the parens.
This is a very minor nit, so I'm ok with keeping the current wording if you
prefer that.
kubabrecka added a comment.
In https://reviews.llvm.org/D27017#611894, @filcab wrote:
> LGTM
>
> (I commented on a minor nit. It might just be me, so feel free to keep the
> current wording if you feel it's preferred)
I'm not seeing this comment. Can you post it again?
filcab accepted this revision.
filcab added a comment.
LGTM
(I commented on a minor nit. It might just be me, so feel free to keep the
current wording if you feel it's preferred)
https://reviews.llvm.org/D27017
___
lldb-commits mailing list
filcab added a comment.
I'd mostly use Anna's suggestions. Kuba: Can you remove the "detected" from the
messages and refresh the patch, just so we can have one last look? Otherwise,
if you think it's good enough, commit with these changes and we can do
post-commit review.
Not going to block the CL over this, but consider using
llvm::StringSwitch<> for improving the readability of this rather large if
/ else block.
On Mon, Nov 28, 2016 at 6:14 PM Anna Zaks via Phabricator via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> zaks.anna added a comment.
>
> Here
filcab added inline comments.
Comment at:
source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp:220
+ } else if (description == "stack-overflow") {
+return "Stack overflow detected (recursion too deep)";
+ } else if (description ==
kubabrecka added a comment.
Thanks for these comments, I'll have them run by more people before committing
this (I'm not a native English speaker). These really need to be as
user-focused as possible, so I'm really against stuff like "not owned pointer",
because that's not something we should
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:
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Looks fine. One question though: do we always have a shared library that
contains the ASAN runtime? If so, then we can actually put the array of short
to long description into the ASAN
kubabrecka created this revision.
kubabrecka added reviewers: clayborg, jasonmolenda, jingham.
kubabrecka added subscribers: lldb-commits, zaks.anna.
kubabrecka set the repository for this revision to rL LLVM.
kubabrecka added a project: Sanitizers.
https://reviews.llvm.org/D27012 will add more
14 matches
Mail list logo
