[PATCH] D52401: Remove redundant null pointer check in operator delete
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
MaskRay added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+ else
+os << "sizeof()";
devnexen wrote:
> MaskRay wrote:
> > Why can't this `else if` case be folded into the `strlcpy` case? There are
> > lots of duplication.
> >
> > `strlcpy` does not check `DstName.empty()` but this one does. Is there any
> > cases I am missing?
> strlcpy does but agreed with your first statement, this handling case for
> both are more different than my initial plan defined them.
Not sure the description of `strlcat` should be different from `strlcpy`... For
both of them, `len` should be less or equal to the size of `dst`. They may
just use the same description.
I think your description of `strlcat` (`"The third argument allows to
potentially copy more bytes than it should. ")` is better while the existing
description of `strlcpy` is problematic:
os << "The third argument is larger than the size of the input buffer. ";
input => output
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
devnexen added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:275
+os << "sizeof(" << DstName << ")";
+ else
+os << "sizeof()";
MaskRay wrote:
> devnexen wrote:
> > MaskRay wrote:
> > > Why can't this `else if` case be folded into the `strlcpy` case? There
> > > are lots of duplication.
> > >
> > > `strlcpy` does not check `DstName.empty()` but this one does. Is there
> > > any cases I am missing?
> > strlcpy does but agreed with your first statement, this handling case for
> > both are more different than my initial plan defined them.
> Not sure the description of `strlcat` should be different from `strlcpy`...
> For both of them, `len` should be less or equal to the size of `dst`. They
> may just use the same description.
>
> I think your description of `strlcat` (`"The third argument allows to
> potentially copy more bytes than it should. ")` is better while the existing
> description of `strlcpy` is problematic:
>
> os << "The third argument is larger than the size of the input buffer. ";
>
> input => output
Fair enough. Code reduction is always nice anyway.
https://reviews.llvm.org/D49722
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
devnexen updated this revision to Diff 166628.
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
--- test/Analysis/cstring-syntax.c
+++ test/Analysis/cstring-syntax.c
@@ -7,6 +7,7 @@
char *strncat(char *, const char *, size_t);
size_t strlen (const char *s);
size_t strlcpy(char *, const char *, size_t);
+size_t strlcat(char *, const char *, size_t);
void testStrncat(const char *src) {
char dest[10];
@@ -27,9 +28,27 @@
strlcpy(dest, src, sizeof(dest));
strlcpy(dest, src, destlen);
strlcpy(dest, src, 10);
- strlcpy(dest, src, 20); // expected-warning {{The third argument is larger than the size of the input buffer. Replace with the value 'sizeof(dest)` or lower}}
- strlcpy(dest, src, badlen); // expected-warning {{The third argument is larger than the size of the input buffer. Replace with the value 'sizeof(dest)` or lower}}
+ strlcpy(dest, src, 20); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+ strlcpy(dest, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
strlcpy(dest, src, ulen);
strlcpy(dest + 5, src, 5);
- strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}}
+ strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+}
+
+void testStrlcat(const char *src) {
+ char dest[10];
+ size_t badlen = 20;
+ size_t ulen;
+ strlcpy(dest, "a", sizeof("a") - 1);
+ strlcat(dest, "", (sizeof("") - 1) - sizeof(dest) - 1);
+ strlcpy(dest, "012345678", sizeof(dest));
+ strlcat(dest, "910", sizeof(dest));
+ strlcpy(dest, "0123456789", sizeof(dest));
+ strlcpy(dest, "0123456789", sizeof(dest));
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+ strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
+ strlcat(dest, src, ulen);
+ strlcpy(dest, src, 5);
+ strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
}
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -90,7 +90,16 @@
/// strlcpy(dst, "abcd", 4);
/// strlcpy(dst + 3, "abcd", 2);
/// strlcpy(dst, "abcd", cpy);
- bool containsBadStrlcpyPattern(const CallExpr *CE);
+ /// Identify erroneous patterns in the last argument to strlcat - the number
+ /// of bytes to copy.
+ /// The bad pattern checked is when the last argument is basically
+ /// pointing to the destination buffer size or argument larger or
+ /// equal to.
+ /// char dst[2];
+ /// strlcat(dst, src2, sizeof(dst));
+ /// strlcat(dst, src2, 2);
+ /// strlcat(dst, src2, 10);
+ bool containsBadStrlcpyStrlcatPattern(const CallExpr *CE);
public:
WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC)
@@ -142,15 +151,19 @@
return false;
}
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
if (CE->getNumArgs() != 3)
return false;
+ const FunctionDecl *FD = CE->getDirectCallee();
+ bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
const auto *DstArgDecl = dyn_cast(DstArg->IgnoreParenImpCasts());
const auto *LenArgDecl = dyn_cast(LenArg->IgnoreParenLValueCasts());
uint64_t DstOff = 0;
+ if (isSizeof(LenArg, DstArg))
+return false;
// - size_t dstlen = sizeof(dst)
if (LenArgDecl) {
const auto *LenArgVal = dyn_cast(LenArgDecl->getDecl());
@@ -181,8 +194,14 @@
if (const auto *Buffer = dyn_cast(DstArgDecl->getType())) {
ASTContext &C = BR.getContext();
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
- return true;
+auto RemainingBufferLen = BufferLen - DstOff;
+if (Append) {
+ if (RemainingBufferLen <= ILRawVal)
+return true;
+} else {
+ if (RemainingBufferLen < ILRawVal)
+return true;
+}
}
}
}
@@ -219,8 +238,9 @@
"C String API", os.str(), Loc,
[PATCH] D52401: Remove redundant null pointer check in operator delete
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
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
void
operator delete(void* ptr) _NOEXCEPT
{
-if (ptr)
-::free(ptr);
+::free(ptr);
}
_LIBCPP_WEAK
@@ -257,11 +256,10 @@
void
operator delete(void* ptr, std::align_val_t) _NOEXCEPT
{
-if (ptr)
#if defined(_LIBCPP_MSVCRT_LIKE)
-::_aligned_free(ptr);
+::_aligned_free(ptr);
#else
-::free(ptr);
+::free(ptr);
#endif
}
Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
void
operator delete(void* ptr) _NOEXCEPT
{
-if (ptr)
-::free(ptr);
+::free(ptr);
}
_LIBCPP_WEAK
@@ -257,11 +256,10 @@
void
operator delete(void* ptr, std::align_val_t) _NOEXCEPT
{
-if (ptr)
#if defined(_LIBCPP_MSVCRT_LIKE)
-::_aligned_free(ptr);
+::_aligned_free(ptr);
#else
-::free(ptr);
+::free(ptr);
#endif
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52331: [Index] Report specialization bases as references when IndexImplicitInstantiation is true
devnexen accepted this revision. devnexen added a comment. This revision is now accepted and ready to land. LGTM to me Repository: rC Clang https://reviews.llvm.org/D52331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52402: [CMake][Fuchsia] Use internal_linkage rather than always_inline for libc++
phosek created this revision.
phosek added reviewers: mcgrathr, jakehehrlich.
Herald added subscribers: cfe-commits, mgorny.
Herald added a reviewer: EricWF.
This is a workaround for PR39053 which was uncovered by
https://reviews.llvm.org/D50652 when
the default attribute has been changed from internal_linkage to
always_inline.
Repository:
rC Clang
https://reviews.llvm.org/D52402
Files:
clang/cmake/caches/Fuchsia-stage2.cmake
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -109,6 +109,8 @@
set(RUNTIMES_${target}-fuchsia_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE
BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF
CACHE BOOL "")
+# TODO: this is a workaround for PR39053 which was uncovered by D50652.
+set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON
CACHE BOOL "")
endforeach()
set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -109,6 +109,8 @@
set(RUNTIMES_${target}-fuchsia_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF CACHE BOOL "")
+# TODO: this is a workaround for PR39053 which was uncovered by D50652.
+set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
endforeach()
set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52402: [CMake][Fuchsia] Use internal_linkage rather than always_inline for libc++
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm as long as we revert when the underlying bugs are resovled Repository: rC Clang https://reviews.llvm.org/D52402 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
MaskRay accepted this revision. MaskRay added a comment. LG https://reviews.llvm.org/D49722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342831 - [Index] Report specialization bases as references when IndexImplicitInstantiation is true
Author: maskray
Date: Sun Sep 23 01:23:48 2018
New Revision: 342831
URL: http://llvm.org/viewvc/llvm-project?rev=342831&view=rev
Log:
[Index] Report specialization bases as references when
IndexImplicitInstantiation is true
Summary:
template struct B {};
template struct D : B {}; // `B` was not reported as a
reference
This patch fixes this.
Reviewers: akyrtzi, arphaman, devnexen
Reviewed By: devnexen
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52331
Modified:
cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
cfe/trunk/test/Index/index-template-specialization.cpp
Modified: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp?rev=342831&r1=342830&r2=342831&view=diff
==
--- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp (original)
+++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp Sun Sep 23 01:23:48 2018
@@ -130,14 +130,15 @@ public:
bool HandleTemplateSpecializationTypeLoc(TypeLocType TL) {
if (const auto *T = TL.getTypePtr()) {
if (IndexCtx.shouldIndexImplicitInstantiation()) {
-if (CXXRecordDecl *RD = T->getAsCXXRecordDecl())
+if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) {
IndexCtx.handleReference(RD, TL.getTemplateNameLoc(),
Parent, ParentDC, SymbolRoleSet(),
Relations);
- } else {
-if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
- IndexCtx.handleReference(D, TL.getTemplateNameLoc(),
- Parent, ParentDC, SymbolRoleSet(),
Relations);
+ return true;
+}
}
+ if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
+IndexCtx.handleReference(D, TL.getTemplateNameLoc(), Parent, ParentDC,
+ SymbolRoleSet(), Relations);
}
return true;
}
Modified: cfe/trunk/test/Index/index-template-specialization.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/index-template-specialization.cpp?rev=342831&r1=342830&r2=342831&view=diff
==
--- cfe/trunk/test/Index/index-template-specialization.cpp (original)
+++ cfe/trunk/test/Index/index-template-specialization.cpp Sun Sep 23 01:23:48
2018
@@ -9,6 +9,12 @@ void g() {
foo.f(0);
}
+template
+struct B {};
+
+template
+struct D : B {};
+
// FIXME: if c-index-test uses OrigD for symbol info, refererences below should
// refer to template specialization decls.
// RUN: env CINDEXTEST_INDEXIMPLICITTEMPLATEINSTANTIATIONS=1 c-index-test
-index-file %s | FileCheck %s
@@ -17,3 +23,7 @@ void g() {
// CHECK-NEXT: [indexDeclaration]: kind: function | name: g
// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: Foo |
USR: c:@ST>1#T@Foo
// CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: f |
USR: c:@ST>1#T@Foo@F@f#t0.0#
+
+// CHECK: [indexDeclaration]: kind: c++-class-template | name: D
+// CHECK-NEXT: : kind: c++-class-template | name: B
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: B
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52331: [Index] Report specialization bases as references when IndexImplicitInstantiation is true
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342831: [Index] Report specialization bases as references
when… (authored by MaskRay, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52331
Files:
cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
cfe/trunk/test/Index/index-template-specialization.cpp
Index: cfe/trunk/test/Index/index-template-specialization.cpp
===
--- cfe/trunk/test/Index/index-template-specialization.cpp
+++ cfe/trunk/test/Index/index-template-specialization.cpp
@@ -9,11 +9,21 @@
foo.f(0);
}
+template
+struct B {};
+
+template
+struct D : B {};
+
// FIXME: if c-index-test uses OrigD for symbol info, refererences below should
// refer to template specialization decls.
// RUN: env CINDEXTEST_INDEXIMPLICITTEMPLATEINSTANTIATIONS=1 c-index-test
-index-file %s | FileCheck %s
// CHECK: [indexDeclaration]: kind: c++-class-template | name: Foo
// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: f
// CHECK-NEXT: [indexDeclaration]: kind: function | name: g
// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: Foo |
USR: c:@ST>1#T@Foo
// CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: f |
USR: c:@ST>1#T@Foo@F@f#t0.0#
+
+// CHECK: [indexDeclaration]: kind: c++-class-template | name: D
+// CHECK-NEXT: : kind: c++-class-template | name: B
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: B
Index: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
===
--- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
+++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
@@ -130,14 +130,15 @@
bool HandleTemplateSpecializationTypeLoc(TypeLocType TL) {
if (const auto *T = TL.getTypePtr()) {
if (IndexCtx.shouldIndexImplicitInstantiation()) {
-if (CXXRecordDecl *RD = T->getAsCXXRecordDecl())
+if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) {
IndexCtx.handleReference(RD, TL.getTemplateNameLoc(),
Parent, ParentDC, SymbolRoleSet(),
Relations);
- } else {
-if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
- IndexCtx.handleReference(D, TL.getTemplateNameLoc(),
- Parent, ParentDC, SymbolRoleSet(),
Relations);
+ return true;
+}
}
+ if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
+IndexCtx.handleReference(D, TL.getTemplateNameLoc(), Parent, ParentDC,
+ SymbolRoleSet(), Relations);
}
return true;
}
Index: cfe/trunk/test/Index/index-template-specialization.cpp
===
--- cfe/trunk/test/Index/index-template-specialization.cpp
+++ cfe/trunk/test/Index/index-template-specialization.cpp
@@ -9,11 +9,21 @@
foo.f(0);
}
+template
+struct B {};
+
+template
+struct D : B {};
+
// FIXME: if c-index-test uses OrigD for symbol info, refererences below should
// refer to template specialization decls.
// RUN: env CINDEXTEST_INDEXIMPLICITTEMPLATEINSTANTIATIONS=1 c-index-test -index-file %s | FileCheck %s
// CHECK: [indexDeclaration]: kind: c++-class-template | name: Foo
// CHECK-NEXT: [indexDeclaration]: kind: c++-instance-method | name: f
// CHECK-NEXT: [indexDeclaration]: kind: function | name: g
// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: Foo | USR: c:@ST>1#T@Foo
// CHECK-NEXT: [indexEntityReference]: kind: c++-instance-method | name: f | USR: c:@ST>1#T@Foo@F@f#t0.0#
+
+// CHECK: [indexDeclaration]: kind: c++-class-template | name: D
+// CHECK-NEXT: : kind: c++-class-template | name: B
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: B
Index: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
===
--- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
+++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp
@@ -130,14 +130,15 @@
bool HandleTemplateSpecializationTypeLoc(TypeLocType TL) {
if (const auto *T = TL.getTypePtr()) {
if (IndexCtx.shouldIndexImplicitInstantiation()) {
-if (CXXRecordDecl *RD = T->getAsCXXRecordDecl())
+if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) {
IndexCtx.handleReference(RD, TL.getTemplateNameLoc(),
Parent, ParentDC, SymbolRoleSet(), Relations);
- } else {
-if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
- IndexCtx.handleReference(D, TL.getTemplateNameLoc(),
- Parent, ParentDC, SymbolRoleSet(), Relations);
+ return true;
+}
}
+ if (const TemplateDecl *D = T->getTemplateName().getAsTemplateDecl())
+
[PATCH] D52401: Remove redundant null pointer check in operator delete
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
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
void
operator delete(void* ptr) _NOEXCEPT
{
-if (ptr)
-::free(ptr);
+::free(ptr);
}
_LIBCPP_WEAK
@@ -257,11 +256,10 @@
void
operator delete(void* ptr, std::align_val_t) _NOEXCEPT
{
-if (ptr)
#if defined(_LIBCPP_MSVCRT_LIKE)
-::_aligned_free(ptr);
+::_aligned_free(ptr);
#else
-::free(ptr);
+::free(ptr);
#endif
}
Index: src/new.cpp
===
--- src/new.cpp
+++ src/new.cpp
@@ -135,8 +135,7 @@
void
operator delete(void* ptr) _NOEXCEPT
{
-if (ptr)
-::free(ptr);
+::free(ptr);
}
_LIBCPP_WEAK
@@ -257,11 +256,10 @@
void
operator delete(void* ptr, std::align_val_t) _NOEXCEPT
{
-if (ptr)
#if defined(_LIBCPP_MSVCRT_LIKE)
-::_aligned_free(ptr);
+::_aligned_free(ptr);
#else
-::free(ptr);
+::free(ptr);
#endif
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342832 - [CStringSyntaxChecker] Check strlcat sizeof check
Author: devnexen
Date: Sun Sep 23 01:30:17 2018
New Revision: 342832
URL: http://llvm.org/viewvc/llvm-project?rev=342832&view=rev
Log:
[CStringSyntaxChecker] Check strlcat sizeof check
Assuming strlcat is used with strlcpy we check as we can if the last argument
does not equal os not larger than the buffer.
Advising the proper usual pattern.
Reviewers: george.karpenkov, NoQ, MaskRay
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D49722
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
cfe/trunk/test/Analysis/cstring-syntax.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp?rev=342832&r1=342831&r2=342832&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp Sun Sep 23
01:30:17 2018
@@ -90,7 +90,16 @@ class WalkAST: public StmtVisitorgetNumArgs() != 3)
return false;
+ const FunctionDecl *FD = CE->getDirectCallee();
+ bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
const auto *DstArgDecl =
dyn_cast(DstArg->IgnoreParenImpCasts());
const auto *LenArgDecl =
dyn_cast(LenArg->IgnoreParenLValueCasts());
uint64_t DstOff = 0;
+ if (isSizeof(LenArg, DstArg))
+return false;
// - size_t dstlen = sizeof(dst)
if (LenArgDecl) {
const auto *LenArgVal = dyn_cast(LenArgDecl->getDecl());
@@ -181,8 +194,14 @@ bool WalkAST::containsBadStrlcpyPattern(
if (const auto *Buffer =
dyn_cast(DstArgDecl->getType())) {
ASTContext &C = BR.getContext();
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
- return true;
+auto RemainingBufferLen = BufferLen - DstOff;
+if (Append) {
+ if (RemainingBufferLen <= ILRawVal)
+return true;
+} else {
+ if (RemainingBufferLen < ILRawVal)
+return true;
+}
}
}
}
@@ -219,8 +238,9 @@ void WalkAST::VisitCallExpr(CallExpr *CE
"C String API", os.str(), Loc,
LenArg->getSourceRange());
}
- } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) {
-if (containsBadStrlcpyPattern(CE)) {
+ } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy") ||
+ CheckerContext::isCLibraryFunction(FD, "strlcat")) {
+if (containsBadStrlcpyStrlcatPattern(CE)) {
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
PathDiagnosticLocation Loc =
@@ -230,13 +250,17 @@ void WalkAST::VisitCallExpr(CallExpr *CE
SmallString<256> S;
llvm::raw_svector_ostream os(S);
- os << "The third argument is larger than the size of the input buffer. ";
+ os << "The third argument allows to potentially copy more bytes than it
should. ";
+ os << "Replace with the value ";
if (!DstName.empty())
-os << "Replace with the value 'sizeof(" << DstName << ")` or lower";
+ os << "sizeof(" << DstName << ")";
+ else
+ os << "sizeof()";
+ os << " or lower";
BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
- "C String API", os.str(), Loc,
- LenArg->getSourceRange());
+ "C String API", os.str(), Loc,
+ LenArg->getSourceRange());
}
}
Modified: cfe/trunk/test/Analysis/cstring-syntax.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cstring-syntax.c?rev=342832&r1=342831&r2=342832&view=diff
==
--- cfe/trunk/test/Analysis/cstring-syntax.c (original)
+++ cfe/trunk/test/Analysis/cstring-syntax.c Sun Sep 23 01:30:17 2018
@@ -7,6 +7,7 @@ typedef __SIZE_TYPE__ size_t;
char *strncat(char *, const char *, size_t);
size_t strlen (const char *s);
size_t strlcpy(char *, const char *, size_t);
+size_t strlcat(char *, const char *, size_t);
void testStrncat(const char *src) {
char dest[10];
@@ -27,9 +28,27 @@ void testStrlcpy(const char *src) {
strlcpy(dest, src, sizeof(dest));
strlcpy(dest, src, destlen);
strlcpy(dest, src, 10);
- strlcpy(dest, src, 20); // expected-warning {{The third argument is larger
than the size of the input buffer. Replace with the value 'sizeof(dest)` or
lower}}
- strlcpy(dest, src, badlen); // expected-warning {{The third argument is
larger than the size of the input buffer. Replace with the value 'sizeof(dest)`
or lower}}
+ strlcpy(dest, src, 20); // expected-warning {{The third argument allows to
potentially copy more bytes than it should. Replace with the value sizeof(des
[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342832: [CStringSyntaxChecker] Check strlcat sizeof check
(authored by devnexen, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D49722
Files:
lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
test/Analysis/cstring-syntax.c
Index: test/Analysis/cstring-syntax.c
===
--- test/Analysis/cstring-syntax.c
+++ test/Analysis/cstring-syntax.c
@@ -7,6 +7,7 @@
char *strncat(char *, const char *, size_t);
size_t strlen (const char *s);
size_t strlcpy(char *, const char *, size_t);
+size_t strlcat(char *, const char *, size_t);
void testStrncat(const char *src) {
char dest[10];
@@ -27,9 +28,27 @@
strlcpy(dest, src, sizeof(dest));
strlcpy(dest, src, destlen);
strlcpy(dest, src, 10);
- strlcpy(dest, src, 20); // expected-warning {{The third argument is larger than the size of the input buffer. Replace with the value 'sizeof(dest)` or lower}}
- strlcpy(dest, src, badlen); // expected-warning {{The third argument is larger than the size of the input buffer. Replace with the value 'sizeof(dest)` or lower}}
+ strlcpy(dest, src, 20); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+ strlcpy(dest, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
strlcpy(dest, src, ulen);
strlcpy(dest + 5, src, 5);
- strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}}
+ strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+}
+
+void testStrlcat(const char *src) {
+ char dest[10];
+ size_t badlen = 20;
+ size_t ulen;
+ strlcpy(dest, "a", sizeof("a") - 1);
+ strlcat(dest, "", (sizeof("") - 1) - sizeof(dest) - 1);
+ strlcpy(dest, "012345678", sizeof(dest));
+ strlcat(dest, "910", sizeof(dest));
+ strlcpy(dest, "0123456789", sizeof(dest));
+ strlcpy(dest, "0123456789", sizeof(dest));
+ strlcat(dest, "0123456789", badlen / 2);
+ strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+ strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
+ strlcat(dest, src, ulen);
+ strlcpy(dest, src, 5);
+ strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
}
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -90,7 +90,16 @@
/// strlcpy(dst, "abcd", 4);
/// strlcpy(dst + 3, "abcd", 2);
/// strlcpy(dst, "abcd", cpy);
- bool containsBadStrlcpyPattern(const CallExpr *CE);
+ /// Identify erroneous patterns in the last argument to strlcat - the number
+ /// of bytes to copy.
+ /// The bad pattern checked is when the last argument is basically
+ /// pointing to the destination buffer size or argument larger or
+ /// equal to.
+ /// char dst[2];
+ /// strlcat(dst, src2, sizeof(dst));
+ /// strlcat(dst, src2, 2);
+ /// strlcat(dst, src2, 10);
+ bool containsBadStrlcpyStrlcatPattern(const CallExpr *CE);
public:
WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC)
@@ -142,15 +151,19 @@
return false;
}
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
if (CE->getNumArgs() != 3)
return false;
+ const FunctionDecl *FD = CE->getDirectCallee();
+ bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
const Expr *DstArg = CE->getArg(0);
const Expr *LenArg = CE->getArg(2);
const auto *DstArgDecl = dyn_cast(DstArg->IgnoreParenImpCasts());
const auto *LenArgDecl = dyn_cast(LenArg->IgnoreParenLValueCasts());
uint64_t DstOff = 0;
+ if (isSizeof(LenArg, DstArg))
+return false;
// - size_t dstlen = sizeof(dst)
if (LenArgDecl) {
const auto *LenArgVal = dyn_cast(LenArgDecl->getDecl());
@@ -181,8 +194,14 @@
if (const auto *Buffer = dyn_cast(DstArgDecl->getType())) {
ASTContext &C = BR.getContext();
uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
- return true;
+auto RemainingBufferLen = BufferLen - DstOff;
+if (Append) {
+ if (RemainingBufferLen <= ILRawVal)
[PATCH] D52401: Remove redundant null pointer check in operator delete
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 note. It is true in C89 (http://port70.net/~nsz/c/c89/c89-draft.html). "If ptr is a null pointer, no action occurs." Also similar wording in POSIX "If ptr is a null pointer, no action shall occur." 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
r342833 - [CMake] Use internal_linkage rather than always_inline for libc++
Author: phosek
Date: Sun Sep 23 01:46:31 2018
New Revision: 342833
URL: http://llvm.org/viewvc/llvm-project?rev=342833&view=rev
Log:
[CMake] Use internal_linkage rather than always_inline for libc++
This is a workaround for PR39053 which was uncovered by D50652 when
the default attribute has been changed from internal_linkage to
always_inline.
Differential Revision: https://reviews.llvm.org/D52402
Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=342833&r1=342832&r2=342833&view=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Sun Sep 23 01:46:31 2018
@@ -109,6 +109,8 @@ if(FUCHSIA_SDK)
set(RUNTIMES_${target}-fuchsia_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE
BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF
CACHE BOOL "")
+# TODO: this is a workaround for PR39053 which was uncovered by D50652.
+set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON
CACHE BOOL "")
endforeach()
set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52402: [CMake][Fuchsia] Use internal_linkage rather than always_inline for libc++
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342833: [CMake] Use internal_linkage rather than
always_inline for libc++ (authored by phosek, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52402?vs=166630&id=166634#toc
Repository:
rC Clang
https://reviews.llvm.org/D52402
Files:
cmake/caches/Fuchsia-stage2.cmake
Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -109,6 +109,8 @@
set(RUNTIMES_${target}-fuchsia_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE
BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF
CACHE BOOL "")
+# TODO: this is a workaround for PR39053 which was uncovered by D50652.
+set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON
CACHE BOOL "")
endforeach()
set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -109,6 +109,8 @@
set(RUNTIMES_${target}-fuchsia_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
set(RUNTIMES_${target}-fuchsia_LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY OFF CACHE BOOL "")
+# TODO: this is a workaround for PR39053 which was uncovered by D50652.
+set(RUNTIMES_${target}-fuchsia_LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "")
endforeach()
set(LLVM_RUNTIME_SANITIZERS "Address" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342834 - [analyzer][UninitializedObjectChecker] Using the new const methods of ImmutableList
Author: szelethus
Date: Sun Sep 23 02:16:27 2018
New Revision: 342834
URL: http://llvm.org/viewvc/llvm-project?rev=342834&view=rev
Log:
[analyzer][UninitializedObjectChecker] Using the new const methods of
ImmutableList
Differential Revision: https://reviews.llvm.org/D51886
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=342834&r1=342833&r2=342834&view=diff
==
---
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
(original)
+++
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
Sun Sep 23 02:16:27 2018
@@ -152,7 +152,6 @@ std::string getVariableName(const FieldD
/// functions such as add() and replaceHead().
class FieldChainInfo {
public:
- using FieldChainImpl = llvm::ImmutableListImpl;
using FieldChain = llvm::ImmutableList;
private:
@@ -179,8 +178,8 @@ public:
bool contains(const FieldRegion *FR) const;
bool isEmpty() const { return Chain.isEmpty(); }
- const FieldRegion *getUninitRegion() const;
- const FieldNode &getHead() { return Chain.getHead(); }
+ const FieldNode &getHead() const { return Chain.getHead(); }
+ const FieldRegion *getUninitRegion() const { return getHead().getRegion(); }
void printNoteMsg(llvm::raw_ostream &Out) const;
};
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=342834&r1=342833&r2=342834&view=diff
==
---
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
(original)
+++
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Sun Sep 23 02:16:27 2018
@@ -339,14 +339,6 @@ bool FindUninitializedFields::isPrimitiv
// Methods for FieldChainInfo.
//===--===//
-const FieldRegion *FieldChainInfo::getUninitRegion() const {
- assert(!Chain.isEmpty() && "Empty fieldchain!");
-
- // ImmutableList::getHead() isn't a const method, hence the not too nice
- // implementation.
- return (*Chain.begin()).getRegion();
-}
-
bool FieldChainInfo::contains(const FieldRegion *FR) const {
for (const FieldNode &Node : Chain) {
if (Node.isSameRegion(FR))
@@ -360,7 +352,7 @@ bool FieldChainInfo::contains(const Fiel
/// recursive function to print the fieldchain correctly. The last element in
/// the chain is to be printed by `FieldChainInfo::print`.
static void printTail(llvm::raw_ostream &Out,
- const FieldChainInfo::FieldChainImpl *L);
+ const FieldChainInfo::FieldChain L);
// FIXME: This function constructs an incorrect string in the following case:
//
@@ -379,8 +371,7 @@ void FieldChainInfo::printNoteMsg(llvm::
if (Chain.isEmpty())
return;
- const FieldChainImpl *L = Chain.getInternalPointer();
- const FieldNode &LastField = L->getHead();
+ const FieldNode &LastField = getHead();
LastField.printNoteMsg(Out);
Out << '\'';
@@ -389,20 +380,20 @@ void FieldChainInfo::printNoteMsg(llvm::
Node.printPrefix(Out);
Out << "this->";
- printTail(Out, L->getTail());
+ printTail(Out, Chain.getTail());
LastField.printNode(Out);
Out << '\'';
}
static void printTail(llvm::raw_ostream &Out,
- const FieldChainInfo::FieldChainImpl *L) {
- if (!L)
+ const FieldChainInfo::FieldChain L) {
+ if (L.isEmpty())
return;
- printTail(Out, L->getTail());
+ printTail(Out, L.getTail());
- L->getHead().printNode(Out);
- L->getHead().printSeparator(Out);
+ L.getHead().printNode(Out);
+ L.getHead().printSeparator(Out);
}
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51886: [analyzer][UninitializedObjectChecker] Using the new const methods of ImmutableList
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342834: [analyzer][UninitializedObjectChecker] Using the new
const methods of… (authored by Szelethus, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D51886
Files:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Index:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
---
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -339,14 +339,6 @@
// Methods for FieldChainInfo.
//===--===//
-const FieldRegion *FieldChainInfo::getUninitRegion() const {
- assert(!Chain.isEmpty() && "Empty fieldchain!");
-
- // ImmutableList::getHead() isn't a const method, hence the not too nice
- // implementation.
- return (*Chain.begin()).getRegion();
-}
-
bool FieldChainInfo::contains(const FieldRegion *FR) const {
for (const FieldNode &Node : Chain) {
if (Node.isSameRegion(FR))
@@ -360,7 +352,7 @@
/// recursive function to print the fieldchain correctly. The last element in
/// the chain is to be printed by `FieldChainInfo::print`.
static void printTail(llvm::raw_ostream &Out,
- const FieldChainInfo::FieldChainImpl *L);
+ const FieldChainInfo::FieldChain L);
// FIXME: This function constructs an incorrect string in the following case:
//
@@ -379,30 +371,29 @@
if (Chain.isEmpty())
return;
- const FieldChainImpl *L = Chain.getInternalPointer();
- const FieldNode &LastField = L->getHead();
+ const FieldNode &LastField = getHead();
LastField.printNoteMsg(Out);
Out << '\'';
for (const FieldNode &Node : Chain)
Node.printPrefix(Out);
Out << "this->";
- printTail(Out, L->getTail());
+ printTail(Out, Chain.getTail());
LastField.printNode(Out);
Out << '\'';
}
static void printTail(llvm::raw_ostream &Out,
- const FieldChainInfo::FieldChainImpl *L) {
- if (!L)
+ const FieldChainInfo::FieldChain L) {
+ if (L.isEmpty())
return;
- printTail(Out, L->getTail());
+ printTail(Out, L.getTail());
- L->getHead().printNode(Out);
- L->getHead().printSeparator(Out);
+ L.getHead().printNode(Out);
+ L.getHead().printSeparator(Out);
}
//===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -152,7 +152,6 @@
/// functions such as add() and replaceHead().
class FieldChainInfo {
public:
- using FieldChainImpl = llvm::ImmutableListImpl;
using FieldChain = llvm::ImmutableList;
private:
@@ -179,8 +178,8 @@
bool contains(const FieldRegion *FR) const;
bool isEmpty() const { return Chain.isEmpty(); }
- const FieldRegion *getUninitRegion() const;
- const FieldNode &getHead() { return Chain.getHead(); }
+ const FieldNode &getHead() const { return Chain.getHead(); }
+ const FieldRegion *getUninitRegion() const { return getHead().getRegion(); }
void printNoteMsg(llvm::raw_ostream &Out) const;
};
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -339,14 +339,6 @@
// Methods for FieldChainInfo.
//===--===//
-const FieldRegion *FieldChainInfo::getUninitRegion() const {
- assert(!Chain.isEmpty() && "Empty fieldchain!");
-
- // ImmutableList::getHead() isn't a const method, hence the not too nice
- // implementation.
- return (*Chain.begin()).getRegion();
-}
-
bool FieldChainInfo::contains(const FieldRegion *FR) const {
for (const FieldNode &Node : Chain) {
if (Node.isSameRegion(FR))
@@ -360,7 +352,7 @@
/// recursive function to print the fieldchain correctly. The last element in
/// the chain is to be printed by `FieldChainInfo::print`.
static void printTail(llvm::raw_ostream &Out,
- const FieldChainInfo::FieldChainImpl *L);
+ const FieldChainInfo::FieldChain L);
// FIXME: This function constructs an incorrect string in the following case:
//
@@ -379,30 +371,29 @@
if (Chain.isEmpty())
return;
- const FieldC
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
theraven added a comment. In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote: > In https://reviews.llvm.org/D52344#1242451, @kristina wrote: > > > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF so > > it's in line with ELF section naming conventions (I'm not entirely sure if > > that could cause issues with ObjC stuff)? And yes I think it's best to > > avoid that code-path altogether if it turns out to be a constant as that's > > likely from the declaration of the type, I'll write a FileCheck test in a > > moment and check that I can apply the same logic to ELF aside from the DLL > > stuff as CoreFoundation needs to export the symbol somehow while preserving > > the implicit extern attribute for every other TU that comes from the > > builtin that `CFSTR` expands to. Is what I'm suggesting making sense? If > > not, let me know, I may be misunderstanding what's happening here. > > > Following the ELF conventions seems like the right thing to do, assuming it > doesn't cause compatibility problems. David Chisnall might know best here. I don't believe it will have any compatibility issues. We expose builtins for creating CF- and NSString objects from C code. For Apple targets, these are equivalent. For targets that don't care about Objective-C interop, the CF version is sensible because it doesn't introduce any dependencies on an Objective-C runtime. For code targeting a non-Apple runtime and wanting CF / Foundation interop, the `CFSTR` macro expands to the NS version. As others have pointed out, the section must be a valid C identifier for the magic `__start_` and `__stop_` symbols to be created. I don't really like this limitation and it would be nice if we could improve lld (and encourage binutils to do the same) so that `__start_.cfstring` and `__stop_.cfstring` could be used to create symbols pointing to the start and end of a section because it's useful to name things in such a way that they can't conflict with any valid C identifier. With PE/COFF, we have a mechanism for more precise control and can give sections any names that we want (and also arrange for start and end symbols for subsections). Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52230: [clang-tidy] use CHECK-NOTES in tests for bugprone-macro-repeated-side-effects
JonasToth added a comment. Friendly ping :) Can i land all the test changes, or do you want to review them? The pattern will be the same, I just want them in different commits to revert better if something changes on different platforms. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52392: [X86] For lzcnt/tzcnt intrinsics use cttz/ctlz intrinsics with zero_undef flag set to false.
RKSimon added a comment. Are there other targets that would benefit from this and if so should we provide a more generic intrinsic? Repository: rC Clang https://reviews.llvm.org/D52392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
On Sun, Sep 23, 2018 at 5:39 AM David Chisnall via Phabricator < revi...@reviews.llvm.org> wrote: > theraven added a comment. > > In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote: > > > In https://reviews.llvm.org/D52344#1242451, @kristina wrote: > > > > > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF > so it's in line with ELF section naming conventions (I'm not entirely sure > if that could cause issues with ObjC stuff)? And yes I think it's best to > avoid that code-path altogether if it turns out to be a constant as that's > likely from the declaration of the type, I'll write a FileCheck test in a > moment and check that I can apply the same logic to ELF aside from the DLL > stuff as CoreFoundation needs to export the symbol somehow while preserving > the implicit extern attribute for every other TU that comes from the > builtin that `CFSTR` expands to. Is what I'm suggesting making sense? If > not, let me know, I may be misunderstanding what's happening here. > > > > > > Following the ELF conventions seems like the right thing to do, assuming > it doesn't cause compatibility problems. David Chisnall might know best > here. > > > I don't believe it will have any compatibility issues. We expose builtins > for creating CF- and NSString objects from C code. For Apple targets, > these are equivalent. For targets that don't care about Objective-C > interop, the CF version is sensible because it doesn't introduce any > dependencies on an Objective-C runtime. For code targeting a non-Apple > runtime and wanting CF / Foundation interop, the `CFSTR` macro expands to > the NS version. > > As others have pointed out, the section must be a valid C identifier for > the magic `__start_` and `__stop_` symbols to be created. I don't really > like this limitation and it would be nice if we could improve lld (and > encourage binutils to do the same) so that `__start_.cfstring` and > `__stop_.cfstring` could be used to create symbols pointing to the start > and end of a section because it's useful to name things in such a way that > they can't conflict with any valid C identifier. With PE/COFF, we have a > mechanism for more precise control and can give sections any names that we > want (and also arrange for start and end symbols for subsections). > Okay. I can respect wanting to change the rules on ELF, but it sounds like we do need to stick with the current section names. Absent an ABI break to stop looking for the existing section names, CF will misbehave if they exist even if we change the compiler output, so effectively those identifiers are claimed in a way that cannot be incrementally changed. Wishing otherwise won't make it so. Also, it's probably correct for CF on non-Darwin OSes to stick to non-system namespaces anyway. John. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342861 - Add inherited attributes before parsed attributes.
Author: meinersbur
Date: Sun Sep 23 23:31:37 2018
New Revision: 342861
URL: http://llvm.org/viewvc/llvm-project?rev=342861&view=rev
Log:
Add inherited attributes before parsed attributes.
Currently, attributes from previous declarations ('inherited attributes')
are added to the end of a declaration's list of attributes. Before
r338800, the attribute list was in reverse. r338800 changed the order
of non-inherited (parsed from the current declaration) attributes, but
inherited attributes are still appended to the end of the list.
This patch appends inherited attributes after other inherited
attributes, but before any non-inherited attribute. This is to make the
order of attributes in the AST correspond to the order in the source
code.
Differential Revision: https://reviews.llvm.org/D50214
Modified:
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/test/Misc/ast-dump-attr.cpp
cfe/trunk/test/Sema/attr-availability-ios.c
cfe/trunk/test/Sema/attr-availability-tvos.c
cfe/trunk/test/Sema/attr-availability-watchos.c
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=342861&r1=342860&r2=342861&view=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Sun Sep 23 23:31:37 2018
@@ -482,13 +482,7 @@ public:
const AttrVec &getAttrs() const;
void dropAttrs();
-
- void addAttr(Attr *A) {
-if (hasAttrs())
- getAttrs().push_back(A);
-else
- setAttrs(AttrVec(1, A));
- }
+ void addAttr(Attr *A);
using attr_iterator = AttrVec::const_iterator;
using attr_range = llvm::iterator_range;
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=342861&r1=342860&r2=342861&view=diff
==
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Sun Sep 23 23:31:37 2018
@@ -836,6 +836,29 @@ void Decl::dropAttrs() {
getASTContext().eraseDeclAttrs(this);
}
+void Decl::addAttr(Attr *A) {
+ if (!hasAttrs()) {
+setAttrs(AttrVec(1, A));
+return;
+ }
+
+ AttrVec &Attrs = getAttrs();
+ if (!A->isInherited()) {
+Attrs.push_back(A);
+return;
+ }
+
+ // Attribute inheritance is processed after attribute parsing. To keep the
+ // order as in the source code, add inherited attributes before non-inherited
+ // ones.
+ auto I = Attrs.begin(), E = Attrs.end();
+ for (; I != E; ++I) {
+if (!(*I)->isInherited())
+ break;
+ }
+ Attrs.insert(I, A);
+}
+
const AttrVec &Decl::getAttrs() const {
assert(HasAttrs && "No attrs to get!");
return getASTContext().getDeclAttrs(this);
Modified: cfe/trunk/test/Misc/ast-dump-attr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-attr.cpp?rev=342861&r1=342860&r2=342861&view=diff
==
--- cfe/trunk/test/Misc/ast-dump-attr.cpp (original)
+++ cfe/trunk/test/Misc/ast-dump-attr.cpp Sun Sep 23 23:31:37 2018
@@ -209,3 +209,24 @@ namespace TestSuppress {
}
}
}
+
+// Verify the order of attributes in the Ast. It must reflect the order
+// in the parsed source.
+int mergeAttrTest() __attribute__((deprecated))
__attribute__((warn_unused_result));
+int mergeAttrTest() __attribute__((annotate("test")));
+int mergeAttrTest() __attribute__((unused,no_thread_safety_analysis));
+// CHECK: FunctionDecl{{.*}} mergeAttrTest
+// CHECK-NEXT: DeprecatedAttr
+// CHECK-NEXT: WarnUnusedResultAttr
+
+// CHECK: FunctionDecl{{.*}} mergeAttrTest
+// CHECK-NEXT: DeprecatedAttr{{.*}} Inherited
+// CHECK-NEXT: WarnUnusedResultAttr{{.*}} Inherited
+// CHECK-NEXT: AnnotateAttr{{.*}}
+
+// CHECK: FunctionDecl{{.*}} mergeAttrTest
+// CHECK-NEXT: DeprecatedAttr{{.*}} Inherited
+// CHECK-NEXT: WarnUnusedResultAttr{{.*}} Inherited
+// CHECK-NEXT: AnnotateAttr{{.*}} Inherited
+// CHECK-NEXT: UnusedAttr
+// CHECK-NEXT: NoThreadSafetyAnalysisAttr
Modified: cfe/trunk/test/Sema/attr-availability-ios.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-availability-ios.c?rev=342861&r1=342860&r2=342861&view=diff
==
--- cfe/trunk/test/Sema/attr-availability-ios.c (original)
+++ cfe/trunk/test/Sema/attr-availability-ios.c Sun Sep 23 23:31:37 2018
@@ -7,8 +7,8 @@ void f3(int) __attribute__((availability
void f4(int)
__attribute__((availability(macosx,introduced=10.1,deprecated=10.3,obsoleted=10.5),
availability(ios,introduced=2.0,deprecated=2.1,obsoleted=3.0))); //
expected-note{{explicitly marked unavailable}}
void f5(int) __attribute__((availability(ios,introduced=2.0)))
__attribute__((availability(ios,deprecated=3.0))); // expected-note {{'f5' has
