[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-10-01 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343503: Remove redundant null pointer check in operator delete (authored by MaskRay, committed by ). Repository: rL LLVM https://reviews.llvm.org/D52401 Files: libcxx/trunk/src/new.cpp Index:

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-10-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. Ok. I'm fine with this. Thanks for your patience. Repository: rL LLVM https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-10-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In https://reviews.llvm.org/D52401#1250552, @MaskRay wrote: > > I seem to recall a problem with that > > I would like to know if your impression came from the common PWN technique > when the attacker found a heap buffer overflow :) No; that's not what I'm looking

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-30 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray added a comment. In https://reviews.llvm.org/D52401#1250551, @MaskRay wrote: > `__free_hook` (defaults to NULL) is a user-supplied hook > (https://www.gnu.org/software/libc/manual/html_node/Hooks-for-Malloc.html). Very interesting, that means if we don't apply this patch, we

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. > I seem to recall a problem with that I would like to know if your impression came from the common PWN technique when the attacker found a heap buffer overflow :) Repository: rL LLVM https://reviews.llvm.org/D52401

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I just checked an extremely old version of glibc fetched from https://ftp.gnu.org/pub/gnu/glibc/ glibc-2.0.1.tar.gz 1997-02-04 03:003.7M `malloc/malloc.c` #if __STD_C void fREe(Void_t* mem) #else void fREe(mem) Void_t* mem; #endif { arena

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I suspect it's fine, but I need to check some stuff on old versions of glibc (I seem to recall a problem with that). Repository: rL LLVM https://reviews.llvm.org/D52401 ___ cfe-commits mailing list

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-28 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray reopened this revision. lichray added a comment. This revision is now accepted and ready to land. LGTM as well, unless @mclow.lists can tell us some history like interactions with K libc :) Repository: rL LLVM https://reviews.llvm.org/D52401

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-24 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. You did not get a thumbs up from any of the code owners for libc++. Reverted in r342938. Repository: rL LLVM https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-24 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL342936: Remove redundant null pointer check in operator delete (authored by MaskRay, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52401

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-24 Thread Yutian Li via Phabricator via cfe-commits
hotpxl accepted this revision. hotpxl added a comment. LGTM Repository: rCXX libc++ https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-24 Thread Ka Ho Ng via Phabricator via cfe-commits
khng300 accepted this revision. khng300 added a comment. This revision is now accepted and ready to land. LGTM Repository: rCXX libc++ https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-24 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping :) Repository: rCXX libc++ https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In https://reviews.llvm.org/D52401#1243054, @ldionne wrote: > Was this true pre-C11 too? If not, then this needs to be guarded by `#if > _LIBCPP_STD_VER >= 17`, because C++ is only on top of C11 in C++17 and above > (Marshall can double-check this). Thanks for the

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 166632. MaskRay edited the summary of this revision. MaskRay added a comment. Quote C89: "If ptr is a null pointer, no action occurs." Repository: rCXX libc++ https://reviews.llvm.org/D52401 Files: src/new.cpp Index: src/new.cpp

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 166629. MaskRay edited the summary of this revision. MaskRay added a comment. Also remove the check for _aligned_free Repository: rCXX libc++ https://reviews.llvm.org/D52401 Files: src/new.cpp Index: src/new.cpp

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-23 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment. Was this true pre-C11 too? If not, then this needs to be guarded by `#if _LIBCPP_STD_VER >= 17`, because C++ is only on top of C11 in C++17 and above (Marshall can double-check this). Repository: rCXX libc++ https://reviews.llvm.org/D52401

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 166625. MaskRay edited the summary of this revision. MaskRay added a comment. . Repository: rCXX libc++ https://reviews.llvm.org/D52401 Files: src/new.cpp Index: src/new.cpp === ---

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-22 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: EricWF, mclow.lists. Herald added subscribers: libcxx-commits, cfe-commits, ldionne, christof. If ptr is a null pointer, no action shall occur. Repository: rCXX libc++ https://reviews.llvm.org/D52401 Files: src/new.cpp Index: