This revision was automatically updated to reflect the committed changes.
Closed by commit rL262894: [analyzer] Fix missed leak from MSVC specific
allocation functions (authored by zaks).
Changed prior to commit:
http://reviews.llvm.org/D17688?vs=49845&id=50013#toc
Repository:
rL LLVM
http:
ariccio added a comment.
In http://reviews.llvm.org/D17688#366780, @zaks.anna wrote:
> LGTM. Thanks!
>
> I can commit this in your behalf.
Oh, and yeah, I don't have privs.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@l
ariccio added a comment.
In http://reviews.llvm.org/D17688#368330, @zaks.anna wrote:
>
?
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ariccio updated this revision to Diff 49845.
ariccio added a comment.
Alrighty. This final version of the patch causes no test failures (vs the
unmodified build*).
*An unrelated test normally fails on my machine.
http://reviews.llvm.org/D17688
Files:
llvm/tools/clang/lib/StaticAnalyzer/Chec
ariccio added a comment.
Hmm. This seems to be because `wchar_t` is enabled by `-fms-compatibility`.
@aaron.ballman do you have any idea how to define `wchar_t` without
`-fms-compatibility`?
http://reviews.llvm.org/D17688
___
cfe-commits mailing l
zaks.anna added a comment.
The second: you should not update the diff. When a patch is uploaded, the
assumption is that all tests pass:)
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
ariccio added a comment.
In http://reviews.llvm.org/D17688#366883, @zaks.anna wrote:
> Please, do not submit patches for review before they pass all tests.
Whoops, sorry. Should I been a bit clearer that check-all hadn't finished when
I updated the diff, or should I not update the diff at all
zaks.anna added a comment.
Please, do not submit patches for review before they pass all tests.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zaks.anna added a comment.
Generate a preprocessed file and keep copying the definitions from there until
you reach the types compiler knows about.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
ariccio added a comment.
Hmm, check-all produced a number of issues, which I think stem from the
following warning:
File C:\LLVM\llvm\tools\clang\test\Analysis\malloc.c Line 16: unknown type
name 'wchar_t'
(with a bunch of instances of that)
...so how do I declare/define `wchar_t` properly?
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
I can commit this in your behalf.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
ariccio updated this revision to Diff 49674.
ariccio added a comment.
Added newly checked variants to the malloc checks.
(Running the check-all suite now, will update with results)
http://reviews.llvm.org/D17688
Files:
llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
llvm/too
ariccio added a comment.
In http://reviews.llvm.org/D17688#366011, @ariccio wrote:
> In http://reviews.llvm.org/D17688#365951, @zaks.anna wrote:
>
> > ls ./clang/test/Analysis/malloc*
>
>
> Ah, ok. Would it be ok if (for _strdup & _alloca) I just do this at the
> beginning:
>
> #if defined(_WI
ariccio added a comment.
In http://reviews.llvm.org/D17688#365951, @zaks.anna wrote:
> ls ./clang/test/Analysis/malloc*
Ah, ok. Would it be ok if (for _strdup & _alloca) I just do this at the
beginning:
#if defined(_WIN32)
#define strdup _strdup
#define alloca _alloca
#endif //d
zaks.anna added a comment.
Of cause, we have regression tests for (almost) everything:
ls ./clang/test/Analysis/malloc*
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
ariccio added a comment.
In http://reviews.llvm.org/D17688#365880, @zaks.anna wrote:
> I suggest to update the malloc regression test with these.
Eh? There's a malloc regression test?
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cf
zaks.anna added a comment.
I suggest to update the malloc regression test with these.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
On Tue, Mar 1, 2016 at 1:42 PM,
wrote:
> I'd quite happily add them... but can I do it in another patch? I think I
> could be more thorough that way.
I'm not certain I understand the reasoning, but I also don't have
strong feelings on whether it's this patch or another. I just don't
understand w
ariccio added a comment.
Is this patch all clear to go?
I hope I don't sound too pushy - I just don't want to lose momentum here.
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
I'd quite happily add them... but can I do it in another patch? I think I
could be more thorough that way.
For the same reason, can we list all the microsoft memory allocating
routines here? There are a thousand routines we might want to add, and then
a few others (like _dupenv_s, _malloca, and _e
On Tue, Mar 1, 2016 at 2:16 AM, Alexander Riccio wrote:
> ariccio updated this revision to Diff 49456.
> ariccio added a comment.
>
> Nit addressed.
>
>
> http://reviews.llvm.org/D17688
>
> Files:
> llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
>
> Index: llvm/tools/clang/lib/St
ariccio updated this revision to Diff 49456.
ariccio added a comment.
Nit addressed.
http://reviews.llvm.org/D17688
Files:
llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Index: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
zaks.anna added a comment.
Nit: Could you change the prefix from "win" to "win_"?
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ariccio added a comment.
//(This is mostly for my own reference)//
BTW, there are a few other non-MS-only functions in the C standard library that
allocate memory that needs to be free()d:
1. getcwd (and _getcwd/_wgetcwd)
And some MS-specific functions that I'm surprised are actually MS-only:
ariccio updated this revision to Diff 49330.
ariccio added a comment.
Changed underscore prefixed variable names to `win` prefixed variable names.
http://reviews.llvm.org/D17688
Files:
llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Index: llvm/tools/clang/lib/StaticAnalyzer/C
ariccio added a comment.
In http://reviews.llvm.org/D17688#363835, @zaks.anna wrote:
> The two underscores in the names are hard to read. Maybe we should just
> prefix them with 'win'?
I like that better, will change.
http://reviews.llvm.org/D17688
On Sun, Feb 28, 2016 at 1:21 PM, Anna Zaks wrote:
> zaks.anna added a comment.
>
> The two underscores in the names are hard to read. Maybe we should just
> prefix them with 'win'?
Two underscores in the name is actually undefined behavior, so I would
second this suggestion. ;-)
~Aaron
zaks.anna added a comment.
The two underscores in the names are hard to read. Maybe we should just prefix
them with 'win'?
http://reviews.llvm.org/D17688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
ariccio created this revision.
ariccio added a subscriber: cfe-commits.
I've found & fixed a leak that Clang misses when compiling on Windows.
The leak was found by [[
https://samate.nist.gov/SARD/view_testcase.php?tID=149071 | SARD #149071 ]],
mem1-bad.c. Clang misses it because MSVC uses _str
30 matches
Mail list logo
