[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added a comment.

Thanks! Yay consistency.

I prefer the term "checker" to refer to individual modules because I feel it is 
more precise and less ambiguous. In phrases like "malloc check", "make_unique 
check", it is unclear what does the check -- malloc itself, the caller of 
malloc, or something else. Therefore, I would be supportive of repainting 
ClangTidy to also use "checker", but I would want to know what @alexfh thinks 
about it before we do it in ClangTidy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67140/new/

https://reviews.llvm.org/D67140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66988: [NewPM][Sancov] Make Sancov a Module Pass instead of 2 Passes

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM

This also makes sense to me why it would OOM. I suspect we're building a 
massive list, and at best we get lucky and they get de-duped later. At worst, 
this is actually fixing a serious functionality problem. We had the same thing 
w/ normal ASan before too. Since this pass doesn't need to be a function pass 
anyways, this seems totally fine. Thanks for tracking it down.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66988/new/

https://reviews.llvm.org/D66988



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yay thanks!!

In my understanding "check" is a feature of the tool, while "checker" is an 
entity within the tool that implements that feature. In most cases there's no 
difference, though, so yeah, it's a tradition thing. I generally enjoy being 
able to say things like "Malloc //checker// (singular!) provides //checks// 
(plural!) for use-after-free bugs and memory leaks". Also i believe that 
"check" in `checkPreCall`  is a verb, so yeah, it should be kept.

> a fundamental difference in between the analyzer and clang-tidy

Honestly, i'm much more worried about message capitalization :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67140/new/

https://reviews.llvm.org/D67140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:133
   std::unique_ptr> InEH;
+  const bool WarnForDeadNestedAssignments;
 

Szelethus wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > I suggest we adopt the idiom of passing the `Checker` object around and 
> > > asking it about its options instead of passing each option around 
> > > separately. This is easier and i don't see any downsides. Moreover, we 
> > > already pass the `Checker` around (just type-erased for no good reason). 
> > > If you don't mind, i'll remove this field as part of resolving merge 
> > > conflicts in D65182.
> > What about subcheckers? In any case, feel free to remove it for now.
> What type erasure do you talk about specifically? In any case, it might 
> happen because of our library layout, I had a bad time with that when I did 
> the checker registration thingie.
I meant `const DeadStoresChecker *` -> `const CheckerBase *`.

> What about subcheckers?

As long as they're represented by the same checker object that's defined within 
the same translation unit, this isn't a problem. When they're multiple checker 
objects - dunno, i've honestly never seen that happen (:

In any case, i don't think it's a hard requirement, just an 
always-easier-thing-to-implement.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66733/new/

https://reviews.llvm.org/D66733



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 218588.
ychen added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65527/new/

https://reviews.llvm.org/D65527

Files:
  clang/test/Driver/verbose-output-quoting.c


Index: clang/test/Driver/verbose-output-quoting.c
===
--- clang/test/Driver/verbose-output-quoting.c
+++ clang/test/Driver/verbose-output-quoting.c
@@ -1,10 +1,10 @@
 // REQUIRES: shell
-// RUN: %clang --verbose -DSPACE="a b"  -c %s 2>&1 | FileCheck 
-check-prefix=SPACE -strict-whitespace %s
-// RUN: %clang --verbose -DQUOTES=\"\"  -c %s 2>&1 | FileCheck 
-check-prefix=QUOTES-strict-whitespace %s
-// RUN: %clang --verbose -DBACKSLASH=\\ -c %s 2>&1 | FileCheck 
-check-prefix=BACKSLASH -strict-whitespace %s
-// RUN: %clang --verbose -DDOLLAR=\$-c %s 2>&1 | FileCheck 
-check-prefix=DOLLAR-strict-whitespace %s
+// RUN: %clang --verbose -DSPACE="a b"  -### %s 2>&1 | FileCheck 
-check-prefix=SPACE -strict-whitespace %s
+// RUN: %clang --verbose -DQUOTES=\"\"  -### %s 2>&1 | FileCheck 
-check-prefix=QUOTES-strict-whitespace %s
+// RUN: %clang --verbose -DBACKSLASH=\\ -### %s 2>&1 | FileCheck 
-check-prefix=BACKSLASH -strict-whitespace %s
+// RUN: %clang --verbose -DDOLLAR=\$-### %s 2>&1 | FileCheck 
-check-prefix=DOLLAR-strict-whitespace %s
 
-// SPACE: -cc1 {{.*}} -D "SPACE=a b"
-// QUOTES: -cc1 {{.*}} -D "QUOTES=\"\""
-// BACKSLASH: -cc1 {{.*}} -D "BACKSLASH=\\"
-// DOLLAR: -cc1 {{.*}} -D "DOLLAR=\$"
+// SPACE: "-cc1" {{.*}} "-D" "SPACE=a b"
+// QUOTES: "-cc1" {{.*}} "-D" "QUOTES=\"\""
+// BACKSLASH: "-cc1" {{.*}} "-D" "BACKSLASH=\\"
+// DOLLAR: "-cc1" {{.*}} "-D" "DOLLAR=\$"


Index: clang/test/Driver/verbose-output-quoting.c
===
--- clang/test/Driver/verbose-output-quoting.c
+++ clang/test/Driver/verbose-output-quoting.c
@@ -1,10 +1,10 @@
 // REQUIRES: shell
-// RUN: %clang --verbose -DSPACE="a b"  -c %s 2>&1 | FileCheck -check-prefix=SPACE -strict-whitespace %s
-// RUN: %clang --verbose -DQUOTES=\"\"  -c %s 2>&1 | FileCheck -check-prefix=QUOTES-strict-whitespace %s
-// RUN: %clang --verbose -DBACKSLASH=\\ -c %s 2>&1 | FileCheck -check-prefix=BACKSLASH -strict-whitespace %s
-// RUN: %clang --verbose -DDOLLAR=\$-c %s 2>&1 | FileCheck -check-prefix=DOLLAR-strict-whitespace %s
+// RUN: %clang --verbose -DSPACE="a b"  -### %s 2>&1 | FileCheck -check-prefix=SPACE -strict-whitespace %s
+// RUN: %clang --verbose -DQUOTES=\"\"  -### %s 2>&1 | FileCheck -check-prefix=QUOTES-strict-whitespace %s
+// RUN: %clang --verbose -DBACKSLASH=\\ -### %s 2>&1 | FileCheck -check-prefix=BACKSLASH -strict-whitespace %s
+// RUN: %clang --verbose -DDOLLAR=\$-### %s 2>&1 | FileCheck -check-prefix=DOLLAR-strict-whitespace %s
 
-// SPACE: -cc1 {{.*}} -D "SPACE=a b"
-// QUOTES: -cc1 {{.*}} -D "QUOTES=\"\""
-// BACKSLASH: -cc1 {{.*}} -D "BACKSLASH=\\"
-// DOLLAR: -cc1 {{.*}} -D "DOLLAR=\$"
+// SPACE: "-cc1" {{.*}} "-D" "SPACE=a b"
+// QUOTES: "-cc1" {{.*}} "-D" "QUOTES=\"\""
+// BACKSLASH: "-cc1" {{.*}} "-D" "BACKSLASH=\\"
+// DOLLAR: "-cc1" {{.*}} "-D" "DOLLAR=\$"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

> Would that work for this test also?

Yes, that sounds good.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65527/new/

https://reviews.llvm.org/D65527



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: akhuang, rnk, probinson.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This essentially reverts changes from r361400.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67141

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/enum2.c


Index: clang/test/CodeGen/enum2.c
===
--- clang/test/CodeGen/enum2.c
+++ clang/test/CodeGen/enum2.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited 
-emit-llvm -o /dev/null
+// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
+
 int v;
 enum e { MAX };
 
@@ -6,3 +7,9 @@
 {
   v = MAX;
 }
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[LONG:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[LONG]] = !DIBasicType(name: "unsigned int", size: 32, encoding: 
DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[MAX:[0-9]+]]}
+// CHECK: ![[MAX]] = !DIEnumerator(name: "MAX", value: 0, isUnsigned: true)
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4438,11 +4438,19 @@
   StringRef Name = VD->getName();
   llvm::DIType *Ty = getOrCreateType(VD->getType(), Unit);
 
-  // Do not use global variables for enums, unless in CodeView.
+  // Use global variables for enums in CodeView, use DW_TAG_enumeration_type 
for
+  // enums for non-CodeView.
   if (const auto *ECD = dyn_cast(VD)) {
 const auto *ED = cast(ECD->getDeclContext());
 assert(isa(ED->getTypeForDecl()) && "Enum without EnumType?");
-(void)ED;
+
+// If not CodeView, emit DW_TAG_enumeration_type if necessary.
+if (!CGM.getCodeGenOpts().EmitCodeView) {
+  llvm::DIType *EDTy =
+  getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
+  if (EDTy->getTag() == llvm::dwarf::DW_TAG_enumeration_type)
+return;
+}
 
 // If CodeView, emit enums as global variables, unless they are defined
 // inside a class. We do this because MSVC doesn't emit S_CONSTANTs for


Index: clang/test/CodeGen/enum2.c
===
--- clang/test/CodeGen/enum2.c
+++ clang/test/CodeGen/enum2.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited -emit-llvm -o /dev/null
+// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+
 int v;
 enum e { MAX };
 
@@ -6,3 +7,9 @@
 {
   v = MAX;
 }
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[LONG:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[LONG]] = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[MAX:[0-9]+]]}
+// CHECK: ![[MAX]] = !DIEnumerator(name: "MAX", value: 0, isUnsigned: true)
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4438,11 +4438,19 @@
   StringRef Name = VD->getName();
   llvm::DIType *Ty = getOrCreateType(VD->getType(), Unit);
 
-  // Do not use global variables for enums, unless in CodeView.
+  // Use global variables for enums in CodeView, use DW_TAG_enumeration_type for
+  // enums for non-CodeView.
   if (const auto *ECD = dyn_cast(VD)) {
 const auto *ED = cast(ECD->getDeclContext());
 assert(isa(ED->getTypeForDecl()) && "Enum without EnumType?");
-(void)ED;
+
+// If not CodeView, emit DW_TAG_enumeration_type if necessary.
+if (!CGM.getCodeGenOpts().EmitCodeView) {
+  llvm::DIType *EDTy =
+  getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
+  if (EDTy->getTag() == llvm::dwarf::DW_TAG_enumeration_type)
+return;
+}
 
 // If CodeView, emit enums as global variables, unless they are defined
 // inside a class. We do this because MSVC doesn't emit S_CONSTANTs for
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:133
   std::unique_ptr> InEH;
+  const bool WarnForDeadNestedAssignments;
 

Szelethus wrote:
> NoQ wrote:
> > I suggest we adopt the idiom of passing the `Checker` object around and 
> > asking it about its options instead of passing each option around 
> > separately. This is easier and i don't see any downsides. Moreover, we 
> > already pass the `Checker` around (just type-erased for no good reason). If 
> > you don't mind, i'll remove this field as part of resolving merge conflicts 
> > in D65182.
> What about subcheckers? In any case, feel free to remove it for now.
What type erasure do you talk about specifically? In any case, it might happen 
because of our library layout, I had a bad time with that when I did the 
checker registration thingie.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66733/new/

https://reviews.llvm.org/D66733



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67091: Fix for headers having the same name as a directory

[Kousik Kumar via Phabricator via cfe-commits]

kousikk added a comment.

Sure I'll add a test case!




Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:196
+  return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
+}
+  }

arphaman wrote:
> This change dropped the createFile call, and didn't fix the issue where the 
> same could happen at the end of the function. Could you please perform this 
> check and return inside of `createFile` instead? This would ensure that both 
> uses are fixed.
Ah sorry, I didn't mean to drop the `createFile()` call. Sure will fix inside 
createFile() instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67091/new/

https://reviews.llvm.org/D67091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added reviewers: gribozavr, alexfh.
Szelethus added subscribers: gribozavr, alexfh.
Szelethus added a comment.

@gribozavr, @alexfh  this isn't a particularly exciting patch, but it does 
highlight a fundamental difference in between the analyzer and clang-tidy, so I 
added you for the sake of it, since we're talking about a closer interaction 
anyway :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67140/new/

https://reviews.llvm.org/D67140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, Charusso, baloghadamsoftware, 
rnkovacs, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

Traditionally, clang-tidy uses the term check, and the analyzer uses checker, 
but in the very early years, this wasn't the case, and code originating from 
the early 2010's still incorrectly refer to checkers as checks.

This patch attempts to hunt down most of these, aiming to refer to checkers as 
checkers, but preserve references to callback functions (like `checkPreCall`) 
as checks.

One thing that is still off in this context (haha) however this 
`CheckerContext`, which a pretty dumb name: it isn't a context of a checker, 
but rather a single callback. Shall I change that as well to `CheckContext`?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67140

Files:
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/include/clang/StaticAnalyzer/Core/Checker.h
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/Checker.cpp
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -30,7 +30,7 @@
 void FlushDiagnosticsImpl(std::vector &Diags,
   FilesMade *filesMade) override {
   for (const auto *PD : Diags)
-Output << PD->getCheckName() << ":" << PD->getShortDescription();
+Output << PD->getCheckerName() << ":" << PD->getShortDescription();
 }
 
 StringRef getName() const override { return "Test"; }
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -437,7 +437,7 @@
 
   // Initialize the CheckerManager with all enabled checkers.
   for (const auto *Checker : enabledCheckers) {
-CheckerMgr.setCurrentCheckName(CheckName(Checker->FullName));
+CheckerMgr.setCurrentCheckerName(CheckerNameRef(Checker->FullName));
 Checker->Initialize(CheckerMgr);
   }
 }
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -627,7 +627,7 @@
 
   if (isBisonFile(C)) {
 reportAnalyzerProgress("Skipping bison-generated file\n");
-  } else if (Opts->DisableAllChecks) {
+  } else if (Opts->DisableAllCheckers) {
 
 // Don't analyze if the user explicitly asked for no checks to be performed
 // on this file.
Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -240,7 +240,7 @@
   const PathPieces &Path = Diag.path.flatten(false);
   const SourceManager &SMgr = Path.front()->getLocation().getManager();
 
-  au

[PATCH] D65182: [analyzer] Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 218575.
NoQ added a comment.

Make fix-it hints attachable to any `PathDiagnosticPiece`, rather than to 
`PathDiagnostic` as a whole. This basically allows attaching a fixit to any 
note in the report. For now the plist consumer only supports attaching fixits 
to the warning itself, to path event pieces, and to extra note pieces; it's 
completely unclear what's the benefit of attaching fixits to other kinds of 
pieces.

Rebase after D66733 .
Fix escaping again.
Address an inline comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65182/new/

https://reviews.llvm.org/D65182

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/dead-stores.c
  clang/test/Analysis/edges-new.mm
  clang/test/Analysis/objc-arc.m
  clang/test/Analysis/plist-output.m
  clang/test/Analysis/virtualcall-fixits.cpp

Index: clang/test/Analysis/virtualcall-fixits.cpp
===
--- /dev/null
+++ clang/test/Analysis/virtualcall-fixits.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN: %s 2>&1 | FileCheck -check-prefix=TEXT %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN: -analyzer-config fixits-as-remarks=true \
+// RUN: -analyzer-output=plist -o %t.plist -verify %s
+// RUN: cat %t.plist | FileCheck -check-prefix=PLIST %s
+
+struct S {
+  virtual void foo();
+  S() {
+foo();
+// expected-warning@-1{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+// expected-remark@-2{{5-5: 'S::'}}
+  }
+  ~S();
+};
+
+// TEXT: warning: Call to virtual method 'S::foo' during construction
+// TEXT-SAME: bypasses virtual dispatch
+// TEXT-NEXT: foo();
+// TEXT-NEXT: ^
+// TEXT-NEXT: S::
+// TEXT-NEXT: 1 warning generated.
+
+// PLIST:  fixits
+// PLIST-NEXT:  
+// PLIST-NEXT:   
+// PLIST-NEXT:remove_range
+// PLIST-NEXT:
+// PLIST-NEXT: 
+// PLIST-NEXT:  line13
+// PLIST-NEXT:  col5
+// PLIST-NEXT:  file0
+// PLIST-NEXT: 
+// PLIST-NEXT: 
+// PLIST-NEXT:  line13
+// PLIST-NEXT:  col4
+// PLIST-NEXT:  file0
+// PLIST-NEXT: 
+// PLIST-NEXT:
+// PLIST-NEXT:insert_stringS::
+// PLIST-NEXT:   
+// PLIST-NEXT:  
Index: clang/test/Analysis/plist-output.m
===
--- clang/test/Analysis/plist-output.m
+++ clang/test/Analysis/plist-output.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist
 // RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/plist-output.m.plist -
 
 void test_null_init(void) {
Index: clang/test/Analysis/objc-arc.m
===
--- clang/test/Analysis/objc-arc.m
+++ clang/test/Analysis/objc-arc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -o %t.plist %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist %s
 // RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/objc-arc.m.plist -
 
 typedef signed char BOOL;
Index: clang/test/Analysis/edges-new.mm
===
--- clang/test/Analysis/edges-new.mm
+++ clang/test/Analysis/edges-new.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,deadcode.DeadStores,osx.cocoa.RetainCount,unix.Malloc,unix.MismatchedDeallocator -analyzer-output=plist -o %t -w %s
+// RUN: %clan

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: 
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp:13
+// variants listed in the RUN script.
+//
+//  Impl   Compile Time  Object Size

zoecarver wrote:
> I think there is a good chance the reason that my added builtin isn't 
> significantly faster is that `std::remove_reference` is already optimized 
> somewhere.  Thoughts?
I think if I updated this test to actually remove a reference, it might show 
more of a difference. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1095
+  default:
+assert(false && "Not a reference type specifier");
+  }

Mordante wrote:
> Wouldn't it be better to use `llvm_unreachable("Not a reference type 
> specifier");`
> Maybe also move this line out of the switch, allowing the compiler to warn 
> about not handled enumeration values.
I'll update it to use `llvm_unreachable` but, I don't think we want the 
compiler to warn about unhandled enumeration values. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

In D67052#1656319 , @mclow.lists wrote:

> If you're going to do `__add__lvalue_reference`, `__add_rvalue_reference`, 
> and `__remove_reference`, why not go all the way and add `__is_reference`, 
> `__is_lvalue_reference` and `__is_rvalue_reference`?


Those already exist :)

I am planning on adding others, `__remove_cv`, `__decay`, etc. but wanted to 
land this first and make sure I was doing it right.

I'll update `type_traits` in libc++ after this lands. I'm also planning on 
making a patch to update a few of the builtin type traits. That should 
significantly speed up build times. FYI here is all the "type trait primitives" 
clang supports. 
 At 
some point, we should try to support all of them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D66361#1656037 , @rsmith wrote:

> In D66361#1655903 , @krytarowski 
> wrote:
>
> > This change broke on NetBSD.
> >
> > http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22003/steps/run%20unit%20tests/logs/FAIL%3A%20Clang%3A%3Astack-exhaustion.cpp
>
>
> Test disabled for NetBSD in r370801. If you're interested in investigating 
> why this isn't working there, feel free, but this is only a best-effort 
> mitigation for the case where things have already gone wrong, so there are 
> limits to how much effort it makes sense to resolve this.
>
> Does NetBSD set a hard stack rlimit of less than 8MB by any chance?


The stack rlimit is restricted by default to 4MB. The maximum at least on amd64 
is 128MB... but if someone really needs it, it could by bypassed with more 
stacks.

  $ ulimit -a
  time(cpu-seconds)unlimited
  file(blocks) unlimited
  coredump(blocks) unlimited
  data(kbytes) 262144
  stack(kbytes)4096
  lockedmem(kbytes)10847213
  memory(kbytes)   32541640
  nofiles(descriptors) 1024
  processes1024
  threads  1024
  vmemory(kbytes)  unlimited
  sbsize(bytes)unlimited

Should the limit by raised by default in the system?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66361/new/

https://reviews.llvm.org/D66361



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67135: Support proto repeated field in performence-inefficient-vector-operation

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please mention new option in Release Notes.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst:9
 
+When EnableProto, also finds calls that add element to protobuf repeated field
+in a loop without calling Reserve() before the loop. Calling Reserve() first

See other checks documentation as example of options descriptions.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67135/new/

https://reviews.llvm.org/D67135



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67127: [clang-scan-deps] add skip excluded conditional preprocessor block preprocessing optimization

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.
Herald added a subscriber: wuzish.



Comment at: clang/include/clang/Frontend/CompilerInstance.h:256
+  /// preprocessor.
+  void setAdditionalPPCallbacks(std::unique_ptr PPC);
+

This kinda sounds like it can be called multiple times.  Is there any way you 
can use chained pp callbacks?



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:171-177
+llvm::cl::opt SkipExcludedPPRanges(
+"skip-excluded-pp-ranges",
+llvm::cl::desc(
+"Use the preprocessor optimization that skips excluded conditionals by 
"
+"bumping the buffer pointer in the lexer instead of lexing the tokens  
"
+"until reaching the end directive."),
+llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));

Is there any reason for this to be configurable other than for perf testing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67127/new/

https://reviews.llvm.org/D67127



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:177
  ``-fsanitize=undefined``.
+  -  ``-fsanitize=pointer-offsetting``: Enables ``nullptr-and-nonzero-offset``
+ and ``pointer-overflow``.

lebedev.ri wrote:
> vsk wrote:
> > Why does this need to be a separate sanitizer, as opposed to being a part 
> > of -fsanitize=pointer-overflow? Is there a need for a new group?
> That ties in with the question i'm asking myself in last update:
> https://reviews.llvm.org/D67122#1656418
> I.e., would it be okay to produce *all* these checks for **every** `gep 
> inbounds` we codegen, indiscriminantly,
> as opposed to only always producing this `nullptr-and-nonzero-offset`?
It's worth considering whether or not to emit a checked GEP on a case-by-case 
basis. For example, there are some GEPs emitted in the ABI lowering logic that 
are not likely to catch bugs in user code. I'm not sure what the point of 
instrumenting these would be.

Separately, the proposed 'nullptr-and-nonzero-offset' check is interesting 
only/exactly when the existing 'pointer-overflow' check is interesting, and 
vice versa. So I don't see the need to make them distinct.



Comment at: compiler-rt/lib/ubsan/ubsan_handlers.cpp:715
+ "applying non-zero offset to non-null pointer %0 producing null "
+ "pointer is undefined behaviour")
+<< (void *)Base;

lebedev.ri wrote:
> vsk wrote:
> > I don't think these diagnostics generally need to repeat the fact that 
> > there is UB: that seems implicit.
> > 
> > How about: 'non-zero offset %0 applied to null pointer', and 'non-zero 
> > offset %0 applied to non-null pointer %1 produced null'?
> > I don't think these diagnostics generally need to repeat the fact that 
> > there is UB: that seems implicit.
> 
> Sure, ok.
> 
> > 'non-zero offset %0 applied to non-null pointer %1 produced null'?
> 
> I don't want to spell any more info that it already does, it isn't really 
> useful //here//.
> Also, we only receive the base pointer and the computed pointer.
> We don't know the actual offset, we may have done `ptr - intptr_t(ptr)`,
> or maybe it was `ptr + (0-intptr_t(ptr))`.
> (Sure, we could receive more info, but i'm not sure worth it here.)
Works for me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 2 inline comments as done.
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:186
+  /// ranges.
+  void addRange(SourceRange R) {
+assert((R.isValid() || Ranges.empty()) && "Invalid range can only be used "

NoQ wrote:
> gribozavr wrote:
> > NoQ wrote:
> > > gribozavr wrote:
> > > > Ranges should be associated with a message.
> > > Mmm, what do you mean?
> > > 
> > > Currently these ranges are attached to the warning message, path note 
> > > messages can't have ranges, and extra path-insensitive notes can have a 
> > > separate set of ranges attached to them by passing them through 
> > > `addNote()`.
> > I see. What looks weird to me is that methods related to the warning itself 
> > are on `BugReport`, but notes and fixits are their own data structures. It 
> > creates an inconsistent API, and makes notes and fixits feel bolted on.
> > 
> > Do you think it would make sense to change the API to be more uniform?
> Hmm, i guess this is an artifact of how path-sensitive checkers usually emits 
> warnings and their respective notes in completely different parts of their 
> code (warnings come from the checker itself, path notes are generated by 
> so-called "bug visitors" which aren't necessarily even a part of the checker).
> 
> Generally we need our notes to be attached to their respective warnings; say, 
> in HTML report they need to be displayed on the same HTML page. But yeah, we 
> should make our APIs more uniform because there's an obvious duplication of 
> effort.
> 
> I also suspect that we'll need a new API in general, because in the current 
> shape the `BugReporter` will look fairly alien and overly-complicated to 
> clang-tidy developers that are used to the conciseness of `diag() << ...`. 
> I'm not sure if it'll boil down to providing convenient wrappers or i'll 
> prefer to rewrite our checkers as well. I think we should talk about this 
> separately on the mailing list.
http://lists.llvm.org/pipermail/cfe-dev/2019-September/063229.html

Basically, `PathDiagnostic` is the uniform API that we've been looking for. 
It's a vector of `PathDiagnosticPiece`s each of which represents a note of 
certain kind (path note, normal note, control flow note, etc.). `BugReporter` 
is a mechanism for converting a `BugReport` into a `PathDiagnostic`. For 
path-sensitive reports this mechanism is extremely sophisticated: the checker 
only supplies a single `ExplodedNode` that corresponds to the end of path and 
the `BugReporter` automatically adds notes (often dozens of them, sometimes 
hundreds) to explain the path. For path-insensitive reports the conversion is 
extremely trivial and therefore there's very little motivation to use the 
`BugReporter` when only path-insensitive reports are expected.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66572/new/

https://reviews.llvm.org/D66572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67135: Support proto repeated field in performence-inefficient-vector-operation

[Cong Liu via Phabricator via cfe-commits]

congliu created this revision.
congliu added a reviewer: gribozavr.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Finds calls that add element to protobuf repeated field in a loop without 
calling Reserve() before the loop. Calling Reserve() first can avoid 
unnecessary memory reallocations.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D67135

Files:
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp

Index: clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
===
--- clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
@@ -1,4 +1,7 @@
-// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- -format-style=llvm
+// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
+// RUN: -format-style=llvm \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
 
 namespace std {
 
@@ -62,6 +65,27 @@
 
 int Op(int);
 
+namespace proto2 {
+class MessageLite {};
+class Message : public MessageLite {};
+} // namespace proto2
+
+class FooProto : public proto2::Message {
+public:
+  int *add_x();
+  void add_x(int x);
+  void mutable_x();
+  void mutable_y();
+};
+
+class BarProto : public proto2::Message {
+public:
+  int *add_x();
+  void add_x(int x);
+  void mutable_x();
+  void mutable_y();
+};
+
 void f(std::vector& t) {
   {
 std::vector v0;
@@ -162,6 +186,24 @@
 }
   }
 
+  {
+FooProto foo;
+// CHECK-FIXES: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'add_x' is called inside a loop; consider pre-allocating the repeated field capacity before the loop
+}
+  }
+  {
+FooProto foo;
+foo.mutable_y();
+// CHECK-FIXES: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'add_x' is called inside a loop
+}
+  }
+
   //  Non-fixed Cases 
   {
 std::vector z0;
@@ -274,4 +316,39 @@
   z12.push_back(e);
 }
   }
+
+  {
+FooProto foo;
+foo.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+foo.add_x(-1);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+BarProto bar;
+bar.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x();
+  bar.add_x();
+}
+  }
 }
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
@@ -6,6 +6,10 @@
 Finds possible inefficient ``std::vector`` operations (e.g. ``push_back``,
 ``emplace_back``) that may cause unnecessary memory reallocations.
 
+When EnableProto, also finds calls that add element to protobuf repeated field
+in a loop without calling Reserve() before the loop. Calling Reserve() first
+can avoid unnecessary memory reallocations.
+
 Currently, the check only detects following kinds of loops with a single
 statement body:
 
@@ -21,6 +25,13 @@
 // statement before the for statement.
   }
 
+  SomeProto p;
+  for (int i = 0; i < n; ++i) {
+p.add_xxx(n);
+// This will trigger the warning since the add_xxx may cause multiple memory
+// relloacations. This can be avoid by inserting a
+// 'p.mutable_xxx().Reserve(n)' statement before the for statement.
+  }
 
 * For-range loops like ``for (range-declaration : range_expression)``, the type
   of ``range_expression`` can be ``std::vector``, ``std::array``,
Index: clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
===
--- clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
+++ clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
@@ -18,6 +18,9 @@
 /// Finds possible inefficient `std::vector` operations (e.g. `push_back`) in
 /// for

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:177
  ``-fsanitize=undefined``.
+  -  ``-fsanitize=pointer-offsetting``: Enables ``nullptr-and-nonzero-offset``
+ and ``pointer-overflow``.

vsk wrote:
> Why does this need to be a separate sanitizer, as opposed to being a part of 
> -fsanitize=pointer-overflow? Is there a need for a new group?
That ties in with the question i'm asking myself in last update:
https://reviews.llvm.org/D67122#1656418
I.e., would it be okay to produce *all* these checks for **every** `gep 
inbounds` we codegen, indiscriminantly,
as opposed to only always producing this `nullptr-and-nonzero-offset`?



Comment at: compiler-rt/lib/ubsan/ubsan_handlers.cpp:715
+ "applying non-zero offset to non-null pointer %0 producing null "
+ "pointer is undefined behaviour")
+<< (void *)Base;

vsk wrote:
> I don't think these diagnostics generally need to repeat the fact that there 
> is UB: that seems implicit.
> 
> How about: 'non-zero offset %0 applied to null pointer', and 'non-zero offset 
> %0 applied to non-null pointer %1 produced null'?
> I don't think these diagnostics generally need to repeat the fact that there 
> is UB: that seems implicit.

Sure, ok.

> 'non-zero offset %0 applied to non-null pointer %1 produced null'?

I don't want to spell any more info that it already does, it isn't really 
useful //here//.
Also, we only receive the base pointer and the computed pointer.
We don't know the actual offset, we may have done `ptr - intptr_t(ptr)`,
or maybe it was `ptr + (0-intptr_t(ptr))`.
(Sure, we could receive more info, but i'm not sure worth it here.)



Comment at: llvm/docs/ReleaseNotes.rst:59
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it

xbolva00 wrote:
> lebedev.ri wrote:
> > xbolva00 wrote:
> > > lebedev.ri wrote:
> > > > xbolva00 wrote:
> > > > > Please add a reference to C/C++ standard that this is a UB.
> > > > It is irrelevant what semantics C/C++ standard has/doesn't have for 
> > > > whatever C/C++ construct,
> > > > this is talking about LLVM IR.
> > > Ok, so Clang docs?
> > > 
> > > “The compiler now optimizes ... since ... is a UB according to ... Use 
> > > UBSan to catch these cases.”
> > ... see `clang/docs/ReleaseNotes.rst` then?
> > Those words were present in the first revision of the differential already,
> > https://reviews.llvm.org/D67122?id=218504#change-RNUd6wICb9iD
> Oh, sorry. 
NP :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 218556.
lebedev.ri marked 7 inline comments as done.
lebedev.ri added a comment.

@vsk thank you for taking a look!
Review notes addressed, except grouping one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-offsetting.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -54,6 +54,20 @@
   ``bcmp`` pattern, and convert it into a call to ``bcmp`` (or ``memcmp``)
   function.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=nullptr-and-nonzero-offset`` check
+  will catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=nullptr-and-nonzero-offset %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O3 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --che

[PATCH] D67113: ICK_Function_Conversion and ICK_Qualification are third kind conversions

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 218550.
aaronpuchert added a comment.

Remove wrong changes in SemaExprCXX.cpp.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67113/new/

https://reviews.llvm.org/D67113

Files:
  clang/lib/Sema/SemaOverload.cpp


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5339,7 +5339,6 @@
   // conversions are fine.
   switch (SCS.Second) {
   case ICK_Identity:
-  case ICK_Function_Conversion:
   case ICK_Integral_Promotion:
   case ICK_Integral_Conversion: // Narrowing conversions are checked elsewhere.
   case ICK_Zero_Queue_Conversion:
@@ -5385,6 +5384,7 @@
   case ICK_Function_To_Pointer:
 llvm_unreachable("found a first conversion kind in Second");
 
+  case ICK_Function_Conversion:
   case ICK_Qualification:
 llvm_unreachable("found a third conversion kind in Second");
 


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5339,7 +5339,6 @@
   // conversions are fine.
   switch (SCS.Second) {
   case ICK_Identity:
-  case ICK_Function_Conversion:
   case ICK_Integral_Promotion:
   case ICK_Integral_Conversion: // Narrowing conversions are checked elsewhere.
   case ICK_Zero_Queue_Conversion:
@@ -5385,6 +5384,7 @@
   case ICK_Function_To_Pointer:
 llvm_unreachable("found a first conversion kind in Second");
 
+  case ICK_Function_Conversion:
   case ICK_Qualification:
 llvm_unreachable("found a third conversion kind in Second");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:177
  ``-fsanitize=undefined``.
+  -  ``-fsanitize=pointer-offsetting``: Enables ``nullptr-and-nonzero-offset``
+ and ``pointer-overflow``.

Why does this need to be a separate sanitizer, as opposed to being a part of 
-fsanitize=pointer-overflow? Is there a need for a new group?



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4544
 
-  // If the GEP has already been reduced to a constant, leave it be.
-  if (isa(GEPVal))
-return GEPVal;
-
-  // Only check for overflows in the default address space.
-  if (GEPVal->getType()->getPointerAddressSpace())
-return GEPVal;
+  // Was the GEP has already been reduced to a constant?
+  if (isa(GEPVal)) {

`s/has//`, `s/been//`



Comment at: clang/lib/CodeGen/CodeGenFunction.h:4072
+  std::pair
+  EmitGEPOffsetInBytes(llvm::Value *BasePtr, llvm::Value *GEPVal);
+

Why does this need to be a method in CodeGenFunction? This seems too tied to 
the work done in EmitCheckedInBoundsGEP to warrant inclusion here.



Comment at: compiler-rt/lib/ubsan/ubsan_handlers.cpp:715
+ "applying non-zero offset to non-null pointer %0 producing null "
+ "pointer is undefined behaviour")
+<< (void *)Base;

I don't think these diagnostics generally need to repeat the fact that there is 
UB: that seems implicit.

How about: 'non-zero offset %0 applied to null pointer', and 'non-zero offset 
%0 applied to non-null pointer %1 produced null'?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:59
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it

lebedev.ri wrote:
> xbolva00 wrote:
> > lebedev.ri wrote:
> > > xbolva00 wrote:
> > > > Please add a reference to C/C++ standard that this is a UB.
> > > It is irrelevant what semantics C/C++ standard has/doesn't have for 
> > > whatever C/C++ construct,
> > > this is talking about LLVM IR.
> > Ok, so Clang docs?
> > 
> > “The compiler now optimizes ... since ... is a UB according to ... Use 
> > UBSan to catch these cases.”
> ... see `clang/docs/ReleaseNotes.rst` then?
> Those words were present in the first revision of the differential already,
> https://reviews.llvm.org/D67122?id=218504#change-RNUd6wICb9iD
Oh, sorry. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66822: Hardware cache line size builtins

[JF Bastien via Phabricator via cfe-commits]

jfb requested changes to this revision.
jfb added a comment.
This revision now requires changes to proceed.

Sorry for the delayed response, I was on vacation. Thanks for tackling it!

I don't think this is the approach I would take. From my dev meeting lightning 
talk I would instead:

1. Add to target infrastructure
2. Overriding in sub-targets using `-march` or `-mcpu`
3. Overriding on the command line
4. If set in target, expose the builtin
5. Generic le32 / be32 ARM targets expose constructive / destructive as 64B
6. Generic le64 / be64 ARM targets expose constructive as 64B and destructive 
as 128B
7. Generic x86 expose constructive / destructive as 64B
8. Honor existing sub-target preferences
9. Let maintainers of other targets choose appropriate size

I think this needs to be split up into a few patches, at least one per target. 
libc++ would then only expose the feature if `__has_builtin` is true. I'm happy 
to go into more details if what I say above is to vague :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66822/new/

https://reviews.llvm.org/D66822



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:59
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it

xbolva00 wrote:
> lebedev.ri wrote:
> > xbolva00 wrote:
> > > Please add a reference to C/C++ standard that this is a UB.
> > It is irrelevant what semantics C/C++ standard has/doesn't have for 
> > whatever C/C++ construct,
> > this is talking about LLVM IR.
> Ok, so Clang docs?
> 
> “The compiler now optimizes ... since ... is a UB according to ... Use UBSan 
> to catch these cases.”
... see `clang/docs/ReleaseNotes.rst` then?
Those words were present in the first revision of the differential already,
https://reviews.llvm.org/D67122?id=218504#change-RNUd6wICb9iD


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 218542.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

`clang/docs/ReleaseNotes.rst`: also mention C17 verse in addition to C++ verse.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-offsetting.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -54,6 +54,20 @@
   ``bcmp`` pattern, and convert it into a call to ``bcmp`` (or ``memcmp``)
   function.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=nullptr-and-nonzero-offset`` check
+  will catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=nullptr-and-nonzero-offset %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer is undefined
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O3 %s -o %t && %run %t 2>&

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:59
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it

lebedev.ri wrote:
> xbolva00 wrote:
> > Please add a reference to C/C++ standard that this is a UB.
> It is irrelevant what semantics C/C++ standard has/doesn't have for whatever 
> C/C++ construct,
> this is talking about LLVM IR.
Ok, so Clang docs?

“The compiler now optimizes ... since ... is a UB according to ... Use UBSan to 
catch these cases.”


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D67023#1654704 , @aaron.ballman 
wrote:

> In D67023#1654070 , @jfb wrote:
>
> > I refer you to http://wg21.link/p0883
> >  I don’t think this diagnostic should be added to clang until p0883 is 
> > fully implemented, even just for C. Otherwise we leave users with no 
> > portable way to do the right thing without diagnostic.
>
>
> P0883 has not been adopted, that I can tell (it has strong support, but isn't 
> [expected to be] a part of C++2a)?


I think it'll make it in as an NB comment.

> Also, this functionality is implemented by libc++ and libstdc++, so I'm not 
> certain what you mean by "until p0883 is fully implemented" or why that paper 
> would be implemented "even just for C". Are you suggesting to gate this 
> deprecation for C functionality based on what C++ standard library 
> implementations are doing?

Yes, because atomics are already hard enough to use correctly between C and 
C++. Once we've fixed C++ I'd like the diagnostic to be the same for both 
(modulo which standard it's deprecated in). I think we should add the 
diagnostic for C and C++ at the same time, and we should make sure there's no 
libc++ nor clang work required before diagnosing.

> I'm sorry if I'm being dense here, but I am still not seeing the issue you're 
> concerned by. C has already deprecated this feature and is planning to remove 
> it shortly. I am diagnosing that fact in C, which seems perfectly appropriate 
> to do regardless of what C++ is doing in this space.
> 
> Users have a portable way to write their code already because 
> `ATOMIC_VAR_INIT` does not do any special magic and no implementation 
> requires a user to use it to achieve portable atomic initialization 
> semantics. If they get the diagnostic (which only triggers in C mode and only 
> if the macro was defined in stdatomic.h), they should remove the use of 
> `ATOMIC_VAR_INIT` from their code, or disable the deprecation diagnostic. If 
> neither of those options appeals to the user, they can write something along 
> the lines of:
> 
>   _Atomic(int) i = 
>   #if defined(__cplusplus)
>   ATOMIC_VAR_INIT(12);
>   #else
>   12;
>   #endif
> 
> 
> (not that I think anyone will want to write that, but strict adherence to a 
> broken part of both standards does not seem like something we want to 
> encourage anyway -- this is deprecated functionality, so the whole point is 
> to discourage its use.)

Is this correct for all implementations of `_Atomic` and `std::atomic` that 
clang supports, including non-lock-free types? We should make sure before 
diagnosing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67023/new/

https://reviews.llvm.org/D67023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 2 inline comments as done.
lebedev.ri added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:59
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it

xbolva00 wrote:
> Please add a reference to C/C++ standard that this is a UB.
It is irrelevant what semantics C/C++ standard has/doesn't have for whatever 
C/C++ construct,
this is talking about LLVM IR.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r370825 - [libclang][test][NFC] Split off fixture from tests.

[Jan Korous via cfe-commits]

Author: jkorous
Date: Tue Sep  3 15:01:46 2019
New Revision: 370825

URL: http://llvm.org/viewvc/llvm-project?rev=370825&view=rev
Log:
[libclang][test][NFC] Split off fixture from tests.

Added:
cfe/trunk/unittests/libclang/TestUtils.h
Modified:
cfe/trunk/unittests/libclang/LibclangTest.cpp

Modified: cfe/trunk/unittests/libclang/LibclangTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/libclang/LibclangTest.cpp?rev=370825&r1=370824&r2=370825&view=diff
==
--- cfe/trunk/unittests/libclang/LibclangTest.cpp (original)
+++ cfe/trunk/unittests/libclang/LibclangTest.cpp Tue Sep  3 15:01:46 2019
@@ -13,6 +13,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
+#include "TestUtils.h"
 #include 
 #include 
 #include 
@@ -352,77 +353,6 @@ TEST(libclang, ModuleMapDescriptor) {
   clang_ModuleMapDescriptor_dispose(MMD);
 }
 
-class LibclangParseTest : public ::testing::Test {
-  std::set Files;
-  typedef std::unique_ptr fixed_addr_string;
-  std::map UnsavedFileContents;
-public:
-  std::string TestDir;
-  CXIndex Index;
-  CXTranslationUnit ClangTU;
-  unsigned TUFlags;
-  std::vector UnsavedFiles;
-
-  void SetUp() override {
-llvm::SmallString<256> Dir;
-ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
-TestDir = Dir.str();
-TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
-  clang_defaultEditingTranslationUnitOptions();
-Index = clang_createIndex(0, 0);
-ClangTU = nullptr;
-  }
-  void TearDown() override {
-clang_disposeTranslationUnit(ClangTU);
-clang_disposeIndex(Index);
-for (const std::string &Path : Files)
-  llvm::sys::fs::remove(Path);
-llvm::sys::fs::remove(TestDir);
-  }
-  void WriteFile(std::string &Filename, const std::string &Contents) {
-if (!llvm::sys::path::is_absolute(Filename)) {
-  llvm::SmallString<256> Path(TestDir);
-  llvm::sys::path::append(Path, Filename);
-  Filename = Path.str();
-  Files.insert(Filename);
-}
-llvm::sys::fs::create_directories(llvm::sys::path::parent_path(Filename));
-std::ofstream OS(Filename);
-OS << Contents;
-assert(OS.good());
-  }
-  void MapUnsavedFile(std::string Filename, const std::string &Contents) {
-if (!llvm::sys::path::is_absolute(Filename)) {
-  llvm::SmallString<256> Path(TestDir);
-  llvm::sys::path::append(Path, Filename);
-  Filename = Path.str();
-}
-auto it = UnsavedFileContents.insert(std::make_pair(
-fixed_addr_string(new std::string(Filename)),
-fixed_addr_string(new std::string(Contents;
-UnsavedFiles.push_back({
-it.first->first->c_str(),   // filename
-it.first->second->c_str(),  // contents
-it.first->second->size()// length
-});
-  }
-  template
-  void Traverse(const F &TraversalFunctor) {
-CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
-std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
-clang_visitChildren(TuCursor,
-&TraverseStateless>,
-&FunctorRef);
-  }
-private:
-  template
-  static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
-  CXClientData data) {
-TState *State = static_cast(data);
-return State->get()(cx, parent);
-  }
-};
-
 TEST_F(LibclangParseTest, AllSkippedRanges) {
   std::string Header = "header.h", Main = "main.cpp";
   WriteFile(Header,

Added: cfe/trunk/unittests/libclang/TestUtils.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/libclang/TestUtils.h?rev=370825&view=auto
==
--- cfe/trunk/unittests/libclang/TestUtils.h (added)
+++ cfe/trunk/unittests/libclang/TestUtils.h Tue Sep  3 15:01:46 2019
@@ -0,0 +1,94 @@
+//===- unittests/libclang/TestUtils.h 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TEST_TESTUTILS_H
+#define LLVM_CLANG_TEST_TESTUTILS_H
+
+#include "clang-c/Index.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+
+#include 
+#include 
+#include 
+#include 
+#include "gtest/gtest.h"
+
+class LibclangParseTest : public ::testing::Test {
+  std::set Files;
+  typedef std::unique_ptr fixed_addr_string;
+  std::map UnsavedFileContents;
+public:
+  std::string TestDir;
+  CXIndex Index;
+  CXTranslationUnit ClangTU;
+  unsigned TUFlags;
+  std::vector UnsavedFiles;
+
+  void SetUp() override {
+llvm::SmallString<256> Dir;
+ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
+TestDir = 

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Can you prepare an NFC patch with just the changes relating to adding 
`ObjCPropertyImplDecl::get{Getter,Setter}MethodDecl`?

I don't get why the redeclaration logic is changing.  What happens when a 
normal method is implemented?  Is that not linked up as a redeclaration?




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:638
 
-void CodeGenFunction::StartFunction(GlobalDecl GD,
-QualType RetTy,
+void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
 llvm::Function *Fn,

Now-spurious style change.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66121/new/

https://reviews.llvm.org/D66121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:59
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it

Please add a reference to C/C++ standard that this is a UB.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:186
+  /// ranges.
+  void addRange(SourceRange R) {
+assert((R.isValid() || Ranges.empty()) && "Invalid range can only be used "

gribozavr wrote:
> NoQ wrote:
> > gribozavr wrote:
> > > Ranges should be associated with a message.
> > Mmm, what do you mean?
> > 
> > Currently these ranges are attached to the warning message, path note 
> > messages can't have ranges, and extra path-insensitive notes can have a 
> > separate set of ranges attached to them by passing them through `addNote()`.
> I see. What looks weird to me is that methods related to the warning itself 
> are on `BugReport`, but notes and fixits are their own data structures. It 
> creates an inconsistent API, and makes notes and fixits feel bolted on.
> 
> Do you think it would make sense to change the API to be more uniform?
Hmm, i guess this is an artifact of how path-sensitive checkers usually emits 
warnings and their respective notes in completely different parts of their code 
(warnings come from the checker itself, path notes are generated by so-called 
"bug visitors" which aren't necessarily even a part of the checker).

Generally we need our notes to be attached to their respective warnings; say, 
in HTML report they need to be displayed on the same HTML page. But yeah, we 
should make our APIs more uniform because there's an obvious duplication of 
effort.

I also suspect that we'll need a new API in general, because in the current 
shape the `BugReporter` will look fairly alien and overly-complicated to 
clang-tidy developers that are used to the conciseness of `diag() << ...`. I'm 
not sure if it'll boil down to providing convenient wrappers or i'll prefer to 
rewrite our checkers as well. I think we should talk about this separately on 
the mailing list.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66572/new/

https://reviews.llvm.org/D66572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 218533.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Reworded `(ptr - intptr_t(ptr)) -> nullptr` ubsan message to be less specific.

Currently, `EmitCheckedInBoundsGEP()` is used sparsely,
a lot of `GEP inbounds` are created directly.
While i suspect that is being done intentionally:

> rL304459 
> 
> - It does not check some GEPs in CGExprCXX. I'm not sure that inserting 
> checks here, or in CGClass, would catch many bugs.

and i think that may have made sense for the original check
(overflow), i'm not sure we get to be this picky here.

I'm still deliberating, but i suspect it will be a good idea
to change all `Builder.Create*InBoundsGEP*()` in clang CodeGen
to go through `EmitCheckedInBoundsGEP()`.
Although i suppose those new cases should only run this sanitizer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-offsetting.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -54,6 +54,20 @@
   ``bcmp`` pattern, and convert it into a call to ``bcmp`` (or ``memcmp``)
   function.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=nullptr-and-nonzero-offset`` check
+  will catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=nullptr-and-nonzero-offset %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer is undefined
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.
Herald added a subscriber: ributzka.

Okay, now that I understand the source-compatibility issues a little better, I 
think this approach is probably okay.  If it causes trouble, we can consider 
special-casing these headers to treat the member as `__unsafe_unretained` 
implicitly — the special case isn't great, but it's better than the potential 
unsoundness.




Comment at: include/clang/AST/ASTContext.h:2060
+  /// attr::ObjCOwnership implies an ownership qualifier was explicitly
+  /// specified rather than being added implicitly by the compiler.
+  bool isObjCOwnershipAttributedType(QualType Ty) const;

How about something like: "Return true if the type has been explicitly 
qualified with ObjC ownership.  A type may be implicitly qualified with 
ownership under ObjC ARC, and in some cases the compiler treats these 
differently".

Could you look for other places where we look for an explicit qualifier?  I'm 
pretty sure I remember that happening once or twice.



Comment at: lib/Sema/SemaDecl.cpp:11176
 for (const FieldDecl *FD : RD->fields())
-  asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+  if (!FD->hasAttr())
+asDerived().visit(FD->getType(), FD, InNonTrivialUnion);

Can we make these exceptions only apply to the attributes we synthesize rather 
than arbitrary uses of `__attribute__((unavailable))`?  These aren't really 
good semantics in general.

You can do the check in a well-named function like 
`isSuppressedNonTrivialMember`, which would be a good place for a comment 
explaining what's going on here and why this seemed like the most reasonable 
solution.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65256/new/

https://reviews.llvm.org/D65256



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65182: [analyzer] Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/test/Analysis/dead-stores.c:11
   long idx=abc+3*5; // expected-warning {{never read}} 
expected-warning{{unused variable 'idx'}}
+  // expected-remark-re@-1.*}}:11 - {{.*}}:18 - ''}}
 }

gribozavr wrote:
> NoQ wrote:
> > gribozavr wrote:
> > > If the tests ignore the line number anyway, why even print it?
> > I anyway hope that this facility is temporary(c).
> What's the eventual replacement?
I hope we eventually learn how to apply fixits automatically, and then 
FileCheck fixed files.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65182/new/

https://reviews.llvm.org/D65182



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65182: [analyzer] Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491
+   ArrayRef Ranges = None,
+   ArrayRef Fixits = None);
 

gribozavr wrote:
> I'm not sure if this is the right model for fixits. Fixits are usually 
> associated with a message that explains what the fixit does. Only in the 
> unusual case where Clang or ClangTidy is very confident that the fixit is 
> correct, it is attached to the warning. Most commonly, fixits are attached to 
> notes.
> 
> Also, for IDE support, it would be really nice if we could provide short 
> descriptions of edits themselves (e.g., "replace 'virtual' with 'override") 
> that can be displayed to the user instead of the diff when possible -- right 
> now we don't and tools using ClangTidy have to use a subpar UI because of 
> that. For example, when we show UI for fixing a warning, displaying the 
> complete diff is too much; a concise description would be a lot better.
> Fixits are usually associated with a message that explains what the fixit 
> does. Only in the unusual case where Clang or ClangTidy is very confident 
> that the fixit is correct, it is attached to the warning.

Wait, you guys already have fixits attached to notes? Then, yeah, i need to 
support this.

> Also, for IDE support, it would be really nice if we could provide short 
> descriptions of edits themselves (e.g., "replace 'virtual' with 'override") 
> that can be displayed to the user instead of the diff when possible -- right 
> now we don't and tools using ClangTidy have to use a subpar UI because of 
> that.

Mmm, interesting. I've seen this IDE of ours autogenerate such messages as 
"replace '`$code_in_removed_range`' with '`$inserted_code`'" (while also 
combining multiple parts of the fixit into a single replacement under the hood) 
and it looked quite bearable most of the time and i silently assumed that 
everybody does the same thing.

I agree that a high-level description of a fixit is nice to have. But given 
that you're attaching fixits to notes, isn't the text of the note text itself 
sufficient? E.g.:
- warning: variable may be used uninitialized here
- note: initialize 'x' here to suppress the warning
  - fixit: add ' = 0' after 'x'


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65182/new/

https://reviews.llvm.org/D65182



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/unittests/AST/ASTImporterVisibilityTest.cpp:343
+::testing::Values(
+std::make_tuple(ExternTypedef, ExternTypedef, ExpectLink),
+std::make_tuple(ExternTypedef, AnonTypedef, ExpectNotLink),

I don't have any objections to changing the names `ExpectLink` and 
`ExpectNotLink` in a subsequent patch but we should update then b/c they are 
confusing (because they are overloaded terms) and it takes some time to work 
backwards to understand the intent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64480/new/

https://reviews.llvm.org/D64480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D64480#1654354 , @balazske wrote:

> In D64480#1653629 , @shafik wrote:
>
> > It is worth noting that:
> >
> >   typedef int T;
> >   typedef int T;
> >
> >
> > is not valid C99 see godbolt 
>
>
> Should we handle this case? This can be special for C99 only when the 
> declarations must be merged instead of linked. Probably this does not cause 
> functional problems if we leave it as is.


I don't think it is critical to handle this case but I was surprised when I 
learned this, it is an edge case and it would be good to note this perhaps as a 
comment in the code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64480/new/

https://reviews.llvm.org/D64480



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67127: [clang-scan-deps] add skip excluded conditional preprocessor block preprocessing optimization

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: Bigcheese, dexonsmith, aganea.
Herald added subscribers: ributzka, tschuett, jsji, jkorous, MaskRay, kbarton, 
mgorny, nemanjai.
Herald added a project: clang.

This patch adds an optimization to clang-scan-deps and clang's preprocessor 
that skips excluded preprocessor blocks by bumping the lexer pointer, and not 
lexing the tokens until reaching appropriate `#else/#endif` directive. The skip 
positions and lexer offsets are computed when the file is minimized, directly 
from the minimized tokens.

On an 18-core iMacPro with macOS Catalina Beta I got 10-15% speed-up from this 
optimization when running `clang-scan-deps` on the compilation database for a 
recent LLVM and Clang (3511 files).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67127

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
  clang/include/clang/Lex/Lexer.h
  clang/include/clang/Lex/PPCallbacks.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/PPRangeSkipping.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/PPRangeSkipping.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -617,4 +617,46 @@
 minimize_source_to_dependency_directives::cxx_module_decl);
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, SkippedPPRangesBasic) {
+  SmallString<128> Out;
+  SmallVector Toks;
+  StringRef Source = "#ifndef GUARD\n"
+ "#define GUARD\n"
+ "void foo();\n"
+ "#endif\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out, Toks));
+  SmallVector Ranges;
+  ASSERT_FALSE(computeSkippedRanges(Toks, Ranges));
+  EXPECT_EQ(Ranges.size(), 1u);
+  EXPECT_EQ(Ranges[0].Offset, 0);
+  EXPECT_EQ(Ranges[0].Length, (int)Out.find("#endif"));
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, SkippedPPRangesNested) {
+  SmallString<128> Out;
+  SmallVector Toks;
+  StringRef Source = "#ifndef GUARD\n"
+ "#define GUARD\n"
+ "#if FOO\n"
+ "#include hello\n"
+ "#elif BAR\n"
+ "#include bye\n"
+ "#endif\n"
+ "#else\n"
+ "#include nothing\n"
+ "#endif\n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out, Toks));
+  SmallVector Ranges;
+  ASSERT_FALSE(computeSkippedRanges(Toks, Ranges));
+  EXPECT_EQ(Ranges.size(), 4u);
+  EXPECT_EQ(Ranges[0].Offset, (int)Out.find("#if FOO"));
+  EXPECT_EQ(Ranges[0].Offset + Ranges[0].Length, (int)Out.find("#elif"));
+  EXPECT_EQ(Ranges[1].Offset, (int)Out.find("#elif BAR"));
+  EXPECT_EQ(Ranges[1].Offset + Ranges[1].Length, (int)Out.find("#endif"));
+  EXPECT_EQ(Ranges[2].Offset, 0);
+  EXPECT_EQ(Ranges[2].Length, (int)Out.find("#else"));
+  EXPECT_EQ(Ranges[3].Offset, (int)Out.find("#else"));
+  EXPECT_EQ(Ranges[3].Offset + Ranges[3].Length, (int)Out.rfind("#endif"));
+}
+
 } // end anonymous namespace
Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -168,6 +168,14 @@
 llvm::cl::desc("Reuse the file manager and its cache between invocations."),
 llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
 
+llvm::cl::opt SkipExcludedPPRanges(
+"skip-excluded-pp-ranges",
+llvm::cl::desc(
+"Use the preprocessor optimization that skips excluded conditionals by "
+"bumping the buffer pointer in the lexer instead of lexing the tokens  "
+"until reaching the end directive."),
+llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
+
 } // end anonymous namespace
 
 int main(int argc, const char **argv) {
@@ -214,7 +222,8 @@
   // Print out the dependency results to STDOU

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:133
   std::unique_ptr> InEH;
+  const bool WarnForDeadNestedAssignments;
 

NoQ wrote:
> I suggest we adopt the idiom of passing the `Checker` object around and 
> asking it about its options instead of passing each option around separately. 
> This is easier and i don't see any downsides. Moreover, we already pass the 
> `Checker` around (just type-erased for no good reason). If you don't mind, 
> i'll remove this field as part of resolving merge conflicts in D65182.
What about subcheckers? In any case, feel free to remove it for now.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66733/new/

https://reviews.llvm.org/D66733



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Louis Dionne via Phabricator via cfe-commits]

ldionne commandeered this revision.
ldionne edited reviewers, added: EricWF; removed: ldionne.
ldionne added a comment.
Herald added a subscriber: jkorous.

Taking over because Eric is on vacation. I think everything has been addressed 
at this point.

@scanon do you see anything else that needs to change?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66836/new/

https://reviews.llvm.org/D66836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:133
   std::unique_ptr> InEH;
+  const bool WarnForDeadNestedAssignments;
 

I suggest we adopt the idiom of passing the `Checker` object around and asking 
it about its options instead of passing each option around separately. This is 
easier and i don't see any downsides. Moreover, we already pass the `Checker` 
around (just type-erased for no good reason). If you don't mind, i'll remove 
this field as part of resolving merge conflicts in D65182.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66733/new/

https://reviews.llvm.org/D66733



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67091: Fix for headers having the same name as a directory

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks for fixing this! Could you add a test case which verifies that the 
assertion no longer happens? Let me know if you need help coming up with a test.




Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:196
+  return 
llvm::ErrorOr>(std::make_error_code(std::errc::is_a_directory));
+}
+  }

This change dropped the createFile call, and didn't fix the issue where the 
same could happen at the end of the function. Could you please perform this 
check and return inside of `createFile` instead? This would ensure that both 
uses are fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67091/new/

https://reviews.llvm.org/D67091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:42
+  return MR->StripCasts();
+}
+

NoQ wrote:
> Charusso wrote:
> > Probably a consistent way of `MemRegion` handling is more appropriate.
> Pointer casts are quite a can of worms. I suggest ignoring it for now and 
> dealing with it as needed. I.e., your code looks good... for now.
> 
> 
//*cries in pointer chasing*//


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67079/new/

https://reviews.llvm.org/D67079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:42
+  return MR->StripCasts();
+}
+

Charusso wrote:
> Probably a consistent way of `MemRegion` handling is more appropriate.
Pointer casts are quite a can of worms. I suggest ignoring it for now and 
dealing with it as needed. I.e., your code looks good... for now.




Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67079/new/

https://reviews.llvm.org/D67079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h:24
+  DynamicTypeInfo(QualType ty, bool CanBeSub = true)
+  : Ty(ty), CanBeASubClass(CanBeSub) {}
 

`Ty(Ty)` is the idiom here.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126
+
+  // If the casts have a common anchestor it could not be a succeeded downcast.
+  for (const auto &PreviousBase : PreviousRD->bases())

Counterexample:

```
struct A {};
struct B : A {};
struct C : A, B {};
```

Downcast from `C` to `B` should succeed, even though they have a common 
ancestor `A` (which has the same `CXXRecordDecl` but currently isn't the same 
object within `C`, but can be, if `B` declares `A` as a virtual base).



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:174
+
+constexpr llvm::StringLiteral Vowels = "aeiou";
+

Omg lol nice. Did you try to figure out how do other people normally do it?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67079/new/

https://reviews.llvm.org/D67079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

If you're going to do `__add__lvalue_reference`, `__add_rvalue_reference`, and 
`__remove_reference`, why not go all the way and add `__is_reference`, 
`__is_lvalue_reference` and `__is_rvalue_reference`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46791: Make -gsplit-dwarf generally available

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich added a comment.

This is obsolete I think. Clang doesn't use llvm-objcopy for this anymore if I 
understand correct. @pcc should be able to confirm.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46791/new/

https://reviews.llvm.org/D46791



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jdoerfert marked an inline comment as done.
Closed by commit rL370817: [Attributor] Deduce "no-capture" argument 
attribute (authored by jdoerfert, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59922?vs=218109&id=218528#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59922/new/

https://reviews.llvm.org/D59922

Files:
  llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
  llvm/trunk/lib/Transforms/IPO/Attributor.cpp
  llvm/trunk/test/Transforms/FunctionAttrs/align.ll
  llvm/trunk/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/trunk/test/Transforms/FunctionAttrs/arg_returned.ll
  llvm/trunk/test/Transforms/FunctionAttrs/dereferenceable.ll
  llvm/trunk/test/Transforms/FunctionAttrs/internal-noalias.ll
  llvm/trunk/test/Transforms/FunctionAttrs/liveness.ll
  llvm/trunk/test/Transforms/FunctionAttrs/noalias_returned.ll
  llvm/trunk/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/trunk/test/Transforms/FunctionAttrs/nosync.ll
  llvm/trunk/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

Index: llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
===
--- llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
+++ llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
@@ -270,6 +270,25 @@
   }
   ///}
 
+  /// Return the associated argument, if any.
+  ///
+  ///{
+  Argument *getAssociatedArgument() {
+if (auto *Arg = dyn_cast(&getAnchorValue()))
+  return Arg;
+int ArgNo = getArgNo();
+if (ArgNo < 0)
+  return nullptr;
+Function *AssociatedFn = getAssociatedFunction();
+if (!AssociatedFn || AssociatedFn->arg_size() <= unsigned(ArgNo))
+  return nullptr;
+return AssociatedFn->arg_begin() + ArgNo;
+  }
+  const Argument *getAssociatedArgument() const {
+return const_cast(this)->getAssociatedArgument();
+  }
+  ///}
+
   /// Return the Function surrounding the anchor value.
   ///
   ///{
@@ -1549,6 +1568,56 @@
   static const char ID;
 };
 
+/// An abstract interface for all nocapture attributes.
+struct AANoCapture
+: public IRAttribute> {
+  AANoCapture(const IRPosition &IRP) : IRAttribute(IRP) {}
+
+  /// State encoding bits. A set bit in the state means the property holds.
+  /// NO_CAPTURE is the best possible state, 0 the worst possible state.
+  enum {
+NOT_CAPTURED_IN_MEM = 1 << 0,
+NOT_CAPTURED_IN_INT = 1 << 1,
+NOT_CAPTURED_IN_RET = 1 << 2,
+
+/// If we do not capture the value in memory or through integers we can only
+/// communicate it back as a derived pointer.
+NO_CAPTURE_MAYBE_RETURNED = NOT_CAPTURED_IN_MEM | NOT_CAPTURED_IN_INT,
+
+/// If we do not capture the value in memory, through integers, or as a
+/// derived pointer we know it is not captured.
+NO_CAPTURE =
+NOT_CAPTURED_IN_MEM | NOT_CAPTURED_IN_INT | NOT_CAPTURED_IN_RET,
+  };
+
+  /// Return true if we know that the underlying value is not captured in its
+  /// respective scope.
+  bool isKnownNoCapture() const { return isKnown(NO_CAPTURE); }
+
+  /// Return true if we assume that the underlying value is not captured in its
+  /// respective scope.
+  bool isAssumedNoCapture() const { return isAssumed(NO_CAPTURE); }
+
+  /// Return true if we know that the underlying value is not captured in its
+  /// respective scope but we allow it to escape through a "return".
+  bool isKnownNoCaptureMaybeReturned() const {
+return isKnown(NO_CAPTURE_MAYBE_RETURNED);
+  }
+
+  /// Return true if we assume that the underlying value is not captured in its
+  /// respective scope but we allow it to escape through a "return".
+  bool isAssumedNoCaptureMaybeReturned() const {
+return isAssumed(NO_CAPTURE_MAYBE_RETURNED);
+  }
+
+  /// Create an abstract attribute view for the position \p IRP.
+  static AANoCapture &createForPosition(const IRPosition &IRP, Attributor &A);
+
+  /// Unique ID (due to the unique address)
+  static const char ID;
+};
+
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_IPO_FUNCTIONATTRS_H
Index: llvm/trunk/test/Transforms/FunctionAttrs/internal-noalias.ll
===
--- llvm/trunk/test/Transforms/FunctionAttrs/internal-noalias.ll
+++ llvm/trunk/test/Transforms/FunctionAttrs/internal-noalias.ll
@@ -10,7 +10,7 @@
 
 ; FIXME: Should be something like this.
 ; define internal i32 @noalias_args(i32* nocapture readonly %A, i32* noalias nocapture readonly %B)
-; CHECK: define internal i32 @noalias_args(i32* %A, i32* %B)
+; CHECK: define internal i32 @noalias_args(i32* nocapture %A, i32* nocapture %B)
 
 define internal i32 @noalias_args(i32* %A, i32* %B) #0 {
 entry:
@@ -25,7 +25,7 @@
 
 ; FIXME: Should be something like this.
 ; define internal i32 @noalias_args_argmem(i32* noalias nocapture readonly %A, i32* noal

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 6 inline comments as done.
jdoerfert added inline comments.



Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2618
+else
+  Attrs.emplace_back(Attribute::get(Ctx, "no-capture-maybe-returned"));
+  }

uenoku wrote:
> Maybe we need an exact definition for `no-capture-maybe-returned` in LangRef.
I add a flag to disable manifestation in non-testing modes. If we add users we 
can add it as a enum attribute, for now it is internal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59922/new/

https://reviews.llvm.org/D59922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66830: Consumed checker: various improvements

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Likely best to separate these changes into separate/standalone patches - easier 
to review/see what's changing, what the motivation is, etc.

(I'm probably OK with some breakage here - to the best of my knowledge these 
attributes haven't achieved widespread adoption, so if it looks like more 
suitable semantics but breaks existing uses a little, that's probably fine)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66830/new/

https://reviews.llvm.org/D66830



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Also, thank you @gribozavr for your comments -- its very nice to have someone 
review a bigger part of our development interface who is knowledgeable about 
Clang, but not the Static Analyzer specifically. Looking at your inlines, these 
are very fair criticisms, I found (find) these confusing when I just started 
contributing as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66572/new/

https://reviews.llvm.org/D66572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46791: Make -gsplit-dwarf generally available

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: test/Driver/split-debug.c:28
+// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
+// MACOSX-CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
 

split dwarf does not make sense on macOS since the debug info is never linked 
into the binary to begin with, so you might as well not test this 
configuration, or even better error out when it is chosen.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46791/new/

https://reviews.llvm.org/D46791



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.

Other then the things @gribozavr mentioned, LGTM.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:458
+public:
+  typedef llvm::SmallVector, 4> ReportList;
+  using iterator = ReportList::iterator;

Prefer `using`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67024/new/

https://reviews.llvm.org/D67024



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:240
+  Arg = Arg->IgnoreImpCasts();
+  if (isa(Arg))
+  Arg = cast(Arg)->getSubExpr();

The bug claims that this is only for handling negative literals, but this 
allows any unary operator. Is that intentional? If we're going to allow 
arbitrary unary operators, why not arbitrary binary operators as well?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67084/new/

https://reviews.llvm.org/D67084



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67052: Add reference type transformation builtins

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1095
+  default:
+assert(false && "Not a reference type specifier");
+  }

Wouldn't it be better to use `llvm_unreachable("Not a reference type 
specifier");`
Maybe also move this line out of the switch, allowing the compiler to warn 
about not handled enumeration values.



Comment at: clang/lib/Sema/SemaType.cpp:1266
+  default:
+assert(false && "Cannot map TST to unary transform type");
+  }

Same here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67052/new/

https://reviews.llvm.org/D67052



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 218519.
lebedev.ri added a comment.

Add test that show that `__builtin_offsetof()` / `((uintptr_t)(&(((S 
*)nullptr)->y)))` are already not sanitized.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-offsetting.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -54,6 +54,20 @@
   ``bcmp`` pattern, and convert it into a call to ``bcmp`` (or ``memcmp``)
   function.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=nullptr-and-nonzero-offset`` check
+  will catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=nullptr-and-nonzero-offset %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer is undefined
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O3 %s -o %t && %run %t 2>&1 | FileCh

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp:15
+  // ERR2: runtime error: pointer index expression with base {{.*}} overflowed 
to
+  // ERR1: runtime error: subtracting integral value of non-null pointer 
0x{{.*}} from pointer itself is undefined
 

Not very friendly error msg since the original code has no sub.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:3581
+llvm::DILocalScope *PrevScope =
+!LexicalBlockStack.empty()
+? dyn_cast(LexicalBlockStack.back())

aganea wrote:
> rnk wrote:
> > Is it OK to look up the lexical block stack at this point? The block stack 
> > isn't function local, it's part of CGDebugInfo, which is for the whole 
> > module, unlike CodeGenFunction. If we start emitting one of these thunks 
> > while we're emitting some other function, we could get some strange 
> > results. Does anything ensure we've pushed at least one scope by the time 
> > we come here?
> You're right, it was looking too much like smart code. Replaced by a new flag 
> `DynamicInitKind::Global` to make explicit the decision to emit an artificial 
> function.
Actually, it is `DynamicInitKind::GlobalDestructor`, sorry about that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:3581
+llvm::DILocalScope *PrevScope =
+!LexicalBlockStack.empty()
+? dyn_cast(LexicalBlockStack.back())

rnk wrote:
> Is it OK to look up the lexical block stack at this point? The block stack 
> isn't function local, it's part of CGDebugInfo, which is for the whole 
> module, unlike CodeGenFunction. If we start emitting one of these thunks 
> while we're emitting some other function, we could get some strange results. 
> Does anything ensure we've pushed at least one scope by the time we come here?
You're right, it was looking too much like smart code. Replaced by a new flag 
`DynamicInitKind::Global` to make explicit the decision to emit an artificial 
function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 218515.
aganea marked 2 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328

Files:
  include/clang/AST/GlobalDecl.h
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDeclCXX.cpp
  test/CodeGenCXX/debug-info-atexit-stub.cpp
  test/CodeGenCXX/debug-info-destroy-helper.cpp
  test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
  test/CodeGenCXX/debug-info-line.cpp

Index: test/CodeGenCXX/debug-info-line.cpp
===
--- test/CodeGenCXX/debug-info-line.cpp
+++ test/CodeGenCXX/debug-info-line.cpp
@@ -314,7 +314,7 @@
 // CHECK: [[DBG_F9]] = !DILocation(line: 1000,
 // CHECK: [[DBG_F10_STORE]] = !DILocation(line: 1100,
 // CHECK: [[DBG_GLBL_CTOR_B]] = !DILocation(line: 1200,
-// CHECK: [[DBG_GLBL_DTOR_B]] = !DILocation(line: 1200,
+// CHECK: [[DBG_GLBL_DTOR_B]] = !DILocation(line: 0,
 // CHECK: [[DBG_F11]] = !DILocation(line: 1300,
 // CHECK: [[DBG_F12]] = !DILocation(line: 1400,
 // CHECK: [[DBG_F13]] = !DILocation(line: 1500,
Index: test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
===
--- test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
+++ test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
@@ -29,25 +29,26 @@
 A FooTpl::sdm_tpl(sizeof(U) + sizeof(T));
 template A FooTpl::sdm_tpl;
 
-// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init",{{.*}} line: 15,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor_glob",{{.*}} line: 15,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init.1",{{.*}} line: 16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} line: 16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor_array",{{.*}} line: 16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor__ZZ3foovE4stat",{{.*}} line: 19,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor_glob",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init.1",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor_array",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor__ZZ3foovE4stat",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
 // CHECK-NOKEXT: !DISubprogram({{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 
 // CHECK-KEXT: !DISubprogram({{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 
-// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'glob'",{{.*}} line: 15,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'glob'",{{.*}} line: 15,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'array'",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'array'",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'stat'",{{.*}} line: 19,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'glob'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'glob'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'array'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'array'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'stat'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
 
 // MSVC does weird stuff when templates are involved, so we don't match exactly,
 

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Jussi Pakkanen via Phabricator via cfe-commits]

jpakkane marked 6 inline comments as done.
jpakkane added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:31-32
+
+  if (!MatchedDecl->isLocalVarDecl())
+return;
+

aaron.ballman wrote:
> jpakkane wrote:
> > aaron.ballman wrote:
> > > I think this should be made into a local AST matcher so the check can be 
> > > hoisted into the matchers.
> > As was discussed earlier, the use of isLocalVarDecl is used here because:
> > 
> > - it has the exact semantics needed in this particular case
> > - it is not available as a matcher, I could not make plain matchers 
> > replicate its behaviour and even if I could, reimplementing it from scratch 
> > seems a bit pointless
> > 
> > If there is a way to use that function directly in the matcher code, let me 
> > know how it's done and I will fix this. But if not, then sadly I have no 
> > idea how that should be fixed and it is probably a bigger development task 
> > than this checker itself.
> You can create your own AST matchers that are used only in this translation 
> unit. e.g.,
> ```
> AST_MATCHER(VarDecl, isLocalVarDecl) {
>   return Node.isLocalVarDecl();
> }
> ```
> and then use this in your AST matchers.
Fixed, thanks.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:59
+  } else if (TypePtr->isFloatingType()) {
+InitializationString = " = (0.0/0.0)"; // NaN without needing #includes
+  } else if (TypePtr->isPointerType()) {

aaron.ballman wrote:
> jpakkane wrote:
> > aaron.ballman wrote:
> > > I would rather see the correct NaN inserted along with the include. See 
> > > StringFindStartswithCheck.cpp for an example of how to use the include 
> > > inserter.
> > I copied the implementation and could make it add the update. However I 
> > could not for the life of me make the test framework accept this. It will 
> > always flag an error even though there should be a comment that declares it 
> > in the test source. Any help is appreciated.
> So the include is inserted as you expect, but the test continues to fail? 
> What does your test look like for it? What failures are you getting?
Just the test in this patch. However after updating the diagnostic code to the 
one recommended made this problem go away. This now works as intended.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:76
+  else if (TypePtr->isFloatingType()) {
+InitializationString = " = NAN";
+AddMathInclude = true;

aaron.ballman wrote:
> In C++11 mode, I think this should recommend 
> `std::numeric_limits::quiet_NaN()` if possible, from `` rather 
> than ``.
As a stylistic point I would disagree with this. In this particular case the 
"C++ way" looks very verbose and confusing. For a single variable this is not a 
huge problem, but it gets very ugly when you have something like this (which is 
common in real world code bases):


```
double d1, d2, d3, d4, d5, d6, d7;
```

This just explodes and is very ugly and unpleasant to read when every one of 
these gets the `std::numeric_limits` template value. I would recommend using 
plain `NAN` for terseness and readability, but can update the PR if the 
template form is deemed the better choice.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64671/new/

https://reviews.llvm.org/D64671



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D66990#1655175 , @ilya-biryukov 
wrote:

> In D66990#1655030 , @nridge wrote:
>
> > I was hoping though that a patch like this, which would bring us largely to 
> > parity with Eclipse CDT's highlightings, wouldn't need to blocked on a 
> > change to the upstream protocol proposal, which could take a while.
>
>
> Is feature parity a hard requirement? Having slightly different highlightings 
> in that case should not be too disruptive.


Not a hard requirement, just a nice-to-have for someone moving from one tool to 
another :)

If you feel that for now it's better not to do this, I can respect that.

>> I think the way cquery does it 
>> 
>>  it better in this regard: in place of a single kind enum, they essentially 
>> have a 4-tuple of `(kind, parent kind, storage class, role)`.
> 
> A design like this definitely makes more sense. I was thinking of a slightly 
> simpler model: adding a set of "modifiers" to each highlighting should be 
> enough to encode all of it, e.g. a modifier 'is class member' could be used 
> to distinguish methods and fields from global functions and variables, a 
> modifier 'is usage' can be used to distinguish usages from declarations, etc.
>  But there's no combinatorial explosion in the cquery's model, which is the 
> important bit.

I will suggest this for the upstream protocol and see where that goes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66990/new/

https://reviews.llvm.org/D66990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D66361: Improve behavior in the case of stack exhaustion.

[Joerg Sonnenberger via cfe-commits]

On Tue, Sep 03, 2019 at 06:02:28PM +, Richard Smith - zygoloid via 
Phabricator via cfe-commits wrote:
> rsmith added a comment.
> 
> In D66361#1655903 , @krytarowski 
> wrote:
> 
> > This change broke on NetBSD.
> >
> > http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22003/steps/run%20unit%20tests/logs/FAIL%3A%20Clang%3A%3Astack-exhaustion.cpp
> 
> 
> Test disabled for NetBSD in r370801. If you're interested in investigating 
> why this isn't working there, feel free, but this is only a best-effort 
> mitigation for the case where things have already gone wrong, so there are 
> limits to how much effort it makes sense to resolve this.
> 
> Does NetBSD set a hard stack rlimit of less than 8MB by any chance?

4MB soft limit by default, hard limit depends on the architecture, e.g.
128MB on amd64.

Joerg
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D67122#1656205 , @lebedev.ri wrote:

> In D67122#1656189 , @aaron.ballman 
> wrote:
>
> > One fear I have with this is in expansions of the  `offsetof` macro, where 
> > it is a common implementation strategy to cast a null pointer to be of the 
> > correct type when calculating member offsets. Do you think you will be able 
> > to distinguish between null pointer additions that the user wrote directly 
> > (which is UB) as opposed to null pointer additions that come from the 
> > implementation (which is not UB)?
>
>
> Can you show a snippet on godbolt?


https://godbolt.org/z/5DHL2E

This will show that Clang has a `__builtin_offsetof()` that gets used. I'm 
worried about situations where there is no `__builtin_offsetof()` but the 
canonical reference implementation is used instead (which looks identical to 
what initializes `bad` in my link).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Jussi Pakkanen via Phabricator via cfe-commits]

jpakkane updated this revision to Diff 218511.
jpakkane added a comment.

Updated patch as per review comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64671/new/

https://reviews.llvm.org/D64671

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

Index: clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp
@@ -0,0 +1,98 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t
+
+#define DO_NOTHING(x) ((void)x)
+
+// Ensure that function declarations are not changed.
+void some_func(int x, double d, bool b, const char *p);
+
+// Ensure that function arguments are not changed
+int identity_function(int x) {
+  return x;
+}
+
+int do_not_modify_me;
+
+typedef struct {
+  int unaltered1;
+  int unaltered2;
+} UnusedStruct;
+
+typedef int my_int_type;
+#define MACRO_INT int
+#define FULL_DECLARATION() int macrodecl;
+
+template 
+void template_test_function() {
+  T t;
+  int uninitialized;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'uninitialized' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int uninitialized = 0;{{$}}
+
+  DO_NOTHING(t);
+  DO_NOTHING(uninitialized);
+}
+
+void init_unit_tests() {
+  int x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'x' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int x = 0;{{$}}
+  my_int_type myint;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'myint' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  my_int_type myint = 0;{{$}}
+
+  MACRO_INT macroint;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'macroint' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  MACRO_INT macroint = 0;{{$}}
+  FULL_DECLARATION();
+
+  int x0 = 1, x1, x2 = 2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'x1' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int x0 = 1, x1 = 0, x2 = 2;{{$}}
+  int y0, y1 = 1, y2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'y0' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: variable 'y2' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int y0 = 0, y1 = 1, y2 = 0;{{$}}
+  int hasval = 42;
+
+  float f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'f' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  float f = NAN;{{$}}
+  float fval = 85.0;
+  double d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: variable 'd' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  double d = NAN;{{$}}
+  double dval = 99.0;
+
+  bool b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable 'b' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  bool b = 0;{{$}}
+  bool bval = true;
+
+  const char *ptr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'ptr' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  const char *ptr = nullptr;{{$}}
+  const char *ptrval = "a string";
+
+  UnusedStruct u;
+
+  DO_NOTHING(x);
+  DO_NOTHING(myint);
+  DO_NOTHING(macroint);
+  DO_NOTHING(macrodecl);
+  DO_NOTHING(x0);
+  DO_NOTHING(x1);
+  DO_NOTHING(x2);
+  DO_NOTHING(y0);
+  DO_NOTHING(y1);
+  DO_NOTHING(y2);
+  DO_NOTHING(hasval);
+  DO_NOTHING(f);
+  DO_NOTHING(fval);
+  DO_NOTHING(d);
+  DO_NOTHING(dval);
+  DO_NOTHING(b);
+  DO_NOTHING(bval);
+  DO_NOTHING(ptr);
+  DO_NOTHING(ptrval);
+  DO_NOTHING(u);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -193,6 +193,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-explicit-virtual-functions (redirects to modernize-use-override) 
+   cppcoreguidelines-init-variables
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi! We've noticed that for our arm bots, we're getting some flaky builds that 
sometimes fail with `error: array designators are a C99 extension 
[-Werror,-Wc99-designator]` and sometimes don't fail. 2 questions:

1. I can't see it off the patch immediately, but do you know why for arm 
specifically we can only get this warning sometimes?
2. I noticed that for the `test/SemaCXX/c99.cpp` test, this warning is also 
diagnosed for the `-std=c++17` case. Are C-style designated initializers only 
invalid in c++20, or are they also invalid in 17?

Thanks.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59754/new/

https://reviews.llvm.org/D59754



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D67122#1656189 , @aaron.ballman 
wrote:

> One fear I have with this is in expansions of the  `offsetof` macro, where it 
> is a common implementation strategy to cast a null pointer to be of the 
> correct type when calculating member offsets. Do you think you will be able 
> to distinguish between null pointer additions that the user wrote directly 
> (which is UB) as opposed to null pointer additions that come from the 
> implementation (which is not UB)?


Can you show a snippet on godbolt?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

One fear I have with this is in expansions of the  `offsetof` macro, where it 
is a common implementation strategy to cast a null pointer to be of the correct 
type when calculating member offsets. Do you think you will be able to 
distinguish between null pointer additions that the user wrote directly (which 
is UB) as opposed to null pointer additions that come from the implementation 
(which is not UB)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67048: [AMDGPU] Set default flat work group size to (1, 256) for HIP

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370808: [AMDGPU] Set default flat work group size to (1,256) 
for HIP (authored by yaxunl, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67048?vs=218261&id=218508#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67048/new/

https://reviews.llvm.org/D67048

Files:
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu


Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -7915,8 +7915,9 @@
 
   const bool IsOpenCLKernel = M.getLangOpts().OpenCL &&
   FD->hasAttr();
-  if ((IsOpenCLKernel ||
-   (M.getLangOpts().HIP && FD->hasAttr())) &&
+  const bool IsHIPKernel = M.getLangOpts().HIP &&
+   FD->hasAttr();
+  if ((IsOpenCLKernel || IsHIPKernel) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
 
@@ -7942,7 +7943,7 @@
   F->addFnAttr("amdgpu-flat-work-group-size", AttrVal);
 } else
   assert(Max == 0 && "Max must be zero");
-  } else if (IsOpenCLKernel) {
+  } else if (IsOpenCLKernel || IsHIPKernel) {
 // By default, restrict the maximum size to 256.
 F->addFnAttr("amdgpu-flat-work-group-size", "1,256");
   }
Index: cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
===
--- cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
+++ cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm -x hip %s -o - | 
FileCheck %s
 #include "Inputs/cuda.h"
 
 // CHECK: define amdgpu_kernel void @_ZN1A6kernelEv
@@ -25,7 +25,7 @@
   EmptyKernelPtr Empty() { return EmptyKernel; } 
 };
 
-// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_
+// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_{{.*}} 
#[[ATTR:[0-9][0-9]*]]
 template
 __global__ void template_kernel(T x) {}
 
@@ -39,3 +39,4 @@
   launch((void*)D.Empty());
   return 0;
 }
+// CHECK: attributes #[[ATTR]] = {{.*}}"amdgpu-flat-work-group-size"="1,256"


Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -7915,8 +7915,9 @@
 
   const bool IsOpenCLKernel = M.getLangOpts().OpenCL &&
   FD->hasAttr();
-  if ((IsOpenCLKernel ||
-   (M.getLangOpts().HIP && FD->hasAttr())) &&
+  const bool IsHIPKernel = M.getLangOpts().HIP &&
+   FD->hasAttr();
+  if ((IsOpenCLKernel || IsHIPKernel) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
 
@@ -7942,7 +7943,7 @@
   F->addFnAttr("amdgpu-flat-work-group-size", AttrVal);
 } else
   assert(Max == 0 && "Max must be zero");
-  } else if (IsOpenCLKernel) {
+  } else if (IsOpenCLKernel || IsHIPKernel) {
 // By default, restrict the maximum size to 256.
 F->addFnAttr("amdgpu-flat-work-group-size", "1,256");
   }
Index: cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
===
--- cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
+++ cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm -x hip %s -o - | FileCheck %s
 #include "Inputs/cuda.h"
 
 // CHECK: define amdgpu_kernel void @_ZN1A6kernelEv
@@ -25,7 +25,7 @@
   EmptyKernelPtr Empty() { return EmptyKernel; } 
 };
 
-// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_
+// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_{{.*}} #[[ATTR:[0-9][0-9]*]]
 template
 __global__ void template_kernel(T x) {}
 
@@ -39,3 +39,4 @@
   launch((void*)D.Empty());
   return 0;
 }
+// CHECK: attributes #[[ATTR]] = {{.*}}"amdgpu-flat-work-group-size"="1,256"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: vsk, filcab, rsmith, aaron.ballman, vitalybuka, 
rjmccall, Sanitizers.
lebedev.ri added projects: clang, Sanitizers.
Herald added subscribers: arphaman, dberris.
Herald added a project: LLVM.

Quote from http://eel.is/c++draft/expr.add#4:

  4 When an expression J that has integral type is added to or subtracted
from an expression P of pointer type, the result has the type of P.
  (4.1) If P evaluates to a null pointer value and J evaluates to 0,
the result is a null pointer value.
  (4.2) Otherwise, if P points to an array element i of an array object x with n
elements ([dcl.array]), the expressions P + J and J + P
(where J has the value j) point to the (possibly-hypothetical) array
element i+j of x if 0≤i+j≤n and the expression P - J points to the 
(possibly-hypothetical) array element i−j of x if 0≤i−j≤n.
  (4.3) Otherwise, the behavior is undefined.

Therefore, as per the standard, applying non-zero offset to `nullptr`
(or making non-`nullptr` a `nullptr`, by subtracting pointer's integral value
from the pointer itself) is undefined behavior. (*if* `nullptr` is not defined,
i.e. e.g. `-fno-delete-null-pointer-checks` was *not* specified.)

Since rL369789  (D66608 
 `[InstCombine] icmp eq/ne (gep inbounds P, 
Idx..), null -> icmp eq/ne P, null`)
LLVM middle-end uses those guarantees for transformations.
If the source contains such UB's, said code may now be miscompiled.
Such miscompilations were already observed:

- https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190826/687838.html
- https://github.com/google/filament/pull/1566

Surprizingly, UBSan does not catch those issues
... until now. This diff teaches UBSan about these UB's.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-offsetting.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -54,6 +54,20 @@
   ``bcmp`` pattern, and convert it into a call to ``bcmp`` (or ``memcmp``)
   function.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize-trap=nullptr-and-nonzero-offset``
+  check will catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=nullptr-and-nonzero-offset %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-inde

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 218506.
craig.topper added a comment.

Change test name and adjust to the changes I think @rksimon was asking for. I 
haven't commited the test yet. I'd like to verify that the before version of 
the test is how you would like it pre-committed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/x86-vec-i128.c

Index: clang/test/CodeGen/x86-vec-i128.c
===
--- clang/test/CodeGen/x86-vec-i128.c
+++ clang/test/CodeGen/x86-vec-i128.c
@@ -3,24 +3,27 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN16,MEM512ALIGN16
 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN32,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN32,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +sse2 -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN32,MEM512ALIGN64
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN32
 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN64
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,CLANG10ABI512
 // RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,CLANG10ABI512
 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
 
 typedef unsigned long long v16u64 __attribute__((vector_size(16)));
 typedef unsigned __int128 v16u128 __attribute__((vector_size(16)));
 
 v16u64 test_v16u128(v16u64 a, v16u128 b) {
-// CLANG10ABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <1 x i128> %{{.*}})
+// CLANG10ABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <2 x i64> %{{.*}})
 // CLANG9ABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <1 x i128> %{{.*}})
   return a + (v16u64)b;
 }
@@ -31,7 +34,7 @@
 v32u64 test_v32u128(v32u64 a, v32u128 b) {
 // MEM256ALIGN16: define <4 x i64> @test_v32u128(<4 x i64>* byval(<4 x i64>) align 16 %{{.*}}, <2 x i128>* byval(<2 x i128>) align 16 %{{.*}})
 // MEM256ALIGN32: define <4 x i64> @test_v32u128(<4 x i64>* byval(<4 x i64>) align 32 %{{.*}}, <2 x i128>* byval(<2 x i128>) align 32 %{{.*}})
-// CLANG10ABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128> %{{.*}})
+// CLANG10ABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128>* byval(<2 x i128>) align 32 %{{.*}})
 // CLANG9ABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128> %{{.*}})
   return a + (v32u64)b;
 }
@@ -43,7 +46,7 @@
 // MEM512ALIGN16: define <8 x i64> @test_v64u128(<8 x i64>* byval(<8 x i64>) align 16 %{{.*}}, <4 x i128>* byval(<4 x i128>) align 16 %{{.*}})
 // MEM512ALIGN32: define <8 x i64> @test_v64u128(<8 x i64>* byval(<8 x i64>) align 32 %{{.*}}, <4 x i128>* byval(<4 x i128>) align 32 %{{.*}})
 // MEM512ALIGN64: define <8 x i64> @test_v64u128(<8 x i64>* by

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 218507.
lebedev.ri added a comment.

NFC, fixup docs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-offsetting.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -54,6 +54,20 @@
   ``bcmp`` pattern, and convert it into a call to ``bcmp`` (or ``memcmp``)
   function.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=nullptr-and-nonzero-offset`` check
+  will catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=nullptr-and-nonzero-offset %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer is undefined
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=nullptr-and-nonzero-offset -O3 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+
+// RUN: %clang -x c++ -fsanitize=nullptr-and-nonzero-offset -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c++ -fsani

r370808 - [AMDGPU] Set default flat work group size to (1, 256) for HIP

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Sep  3 11:50:24 2019
New Revision: 370808

URL: http://llvm.org/viewvc/llvm-project?rev=370808&view=rev
Log:
[AMDGPU] Set default flat work group size to (1,256) for HIP

Differential Revision: https://reviews.llvm.org/D67048

Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=370808&r1=370807&r2=370808&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Sep  3 11:50:24 2019
@@ -7915,8 +7915,9 @@ void AMDGPUTargetCodeGenInfo::setTargetA
 
   const bool IsOpenCLKernel = M.getLangOpts().OpenCL &&
   FD->hasAttr();
-  if ((IsOpenCLKernel ||
-   (M.getLangOpts().HIP && FD->hasAttr())) &&
+  const bool IsHIPKernel = M.getLangOpts().HIP &&
+   FD->hasAttr();
+  if ((IsOpenCLKernel || IsHIPKernel) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
 
@@ -7942,7 +7943,7 @@ void AMDGPUTargetCodeGenInfo::setTargetA
   F->addFnAttr("amdgpu-flat-work-group-size", AttrVal);
 } else
   assert(Max == 0 && "Max must be zero");
-  } else if (IsOpenCLKernel) {
+  } else if (IsOpenCLKernel || IsHIPKernel) {
 // By default, restrict the maximum size to 256.
 F->addFnAttr("amdgpu-flat-work-group-size", "1,256");
   }

Modified: cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu?rev=370808&r1=370807&r2=370808&view=diff
==
--- cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu Tue Sep  3 11:50:24 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm -x hip %s -o - | 
FileCheck %s
 #include "Inputs/cuda.h"
 
 // CHECK: define amdgpu_kernel void @_ZN1A6kernelEv
@@ -25,7 +25,7 @@ struct Dummy {
   EmptyKernelPtr Empty() { return EmptyKernel; } 
 };
 
-// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_
+// CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_{{.*}} 
#[[ATTR:[0-9][0-9]*]]
 template
 __global__ void template_kernel(T x) {}
 
@@ -39,3 +39,4 @@ int main() {
   launch((void*)D.Empty());
   return 0;
 }
+// CHECK: attributes #[[ATTR]] = {{.*}}"amdgpu-flat-work-group-size"="1,256"


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Thomas Lively via Phabricator via cfe-commits]

tlively abandoned this revision.
tlively added a comment.

Abandoning in favor of @sunfish's idea for introducing a cost mechanism for 
shuffle masks in DAGCombiner.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66983/new/

https://reviews.llvm.org/D66983



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r370807 - Unbreak the build after r370798

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Sep  3 11:24:56 2019
New Revision: 370807

URL: http://llvm.org/viewvc/llvm-project?rev=370807&view=rev
Log:
Unbreak the build after r370798

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp?rev=370807&r1=370806&r2=370807&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp Tue Sep  3 
11:24:56 2019
@@ -196,7 +196,7 @@ NonNullParamChecker::genReportNullAttrNo
  << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg)
  << " parameter expecting 'nonnull'";
 
-  auto R = llvm::make_unique(*BTAttrNonNull, SBuf, ErrorNode);
+  auto R = std::make_unique(*BTAttrNonNull, SBuf, ErrorNode);
   if (ArgE)
 bugreporter::trackExpressionValue(ErrorNode, ArgE, *R);
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67119: On PowerPC, Secure-PLT by default for FreeBSD 13 and higher

[Dimitry Andric via Phabricator via cfe-commits]

dim created this revision.
dim added reviewers: emaste, jhibbits, hfinkel.
Herald added subscribers: steven.zhang, shchenz, jsji, MaskRay, kbarton, 
krytarowski, nemanjai.
Herald added a project: clang.

In https://svnweb.freebsd.org/changeset/base/349351, FreeBSD 13 and
higher transitioned to Secure-PLT for PowerPC.  This part contains the
changes in clang's PPC architecture defaults.


Repository:
  rC Clang

https://reviews.llvm.org/D67119

Files:
  lib/Driver/ToolChains/Arch/PPC.cpp


Index: lib/Driver/ToolChains/Arch/PPC.cpp
===
--- lib/Driver/ToolChains/Arch/PPC.cpp
+++ lib/Driver/ToolChains/Arch/PPC.cpp
@@ -115,7 +115,8 @@
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if (Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
+  if ((Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 13) ||
+  Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;


Index: lib/Driver/ToolChains/Arch/PPC.cpp
===
--- lib/Driver/ToolChains/Arch/PPC.cpp
+++ lib/Driver/ToolChains/Arch/PPC.cpp
@@ -115,7 +115,8 @@
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if (Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
+  if ((Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 13) ||
+  Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66988: [NewPM][Sancov] Make Sancov a Module Pass instead of 2 Passes

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66988/new/

https://reviews.llvm.org/D66988



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46791: Make -gsplit-dwarf generally available

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D46791/new/

https://reviews.llvm.org/D46791



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67117: [clangd] Split Preamble.h out of ClangdUnit.h. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67117/new/

https://reviews.llvm.org/D67117



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D66361#1655903 , @krytarowski wrote:

> This change broke on NetBSD.
>
> http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22003/steps/run%20unit%20tests/logs/FAIL%3A%20Clang%3A%3Astack-exhaustion.cpp


Test disabled for NetBSD in r370801. If you're interested in investigating why 
this isn't working there, feel free, but this is only a best-effort mitigation 
for the case where things have already gone wrong, so there are limits to how 
much effort it makes sense to resolve this.

Does NetBSD set a hard stack rlimit of less than 8MB by any chance?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66361/new/

https://reviews.llvm.org/D66361



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r370801 - Disable stack exhaustion test on NetBSD, where either the detection or

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Sep  3 11:00:44 2019
New Revision: 370801

URL: http://llvm.org/viewvc/llvm-project?rev=370801&view=rev
Log:
Disable stack exhaustion test on NetBSD, where either the detection or
recovery mechanism does not appear to work.

Modified:
cfe/trunk/test/SemaTemplate/stack-exhaustion.cpp

Modified: cfe/trunk/test/SemaTemplate/stack-exhaustion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/stack-exhaustion.cpp?rev=370801&r1=370800&r2=370801&view=diff
==
--- cfe/trunk/test/SemaTemplate/stack-exhaustion.cpp (original)
+++ cfe/trunk/test/SemaTemplate/stack-exhaustion.cpp Tue Sep  3 11:00:44 2019
@@ -1,6 +1,11 @@
 // RUN: %clang_cc1 -verify %s
 // REQUIRES: thread_support
 
+// FIXME: Detection of, or recovery from, stack exhaustion does not work on
+// NetBSD at the moment. Since this is a best-effort mitigation for exceeding
+// implementation limits, just disable the test.
+// UNSUPPORTED: system-netbsd
+
 // expected-warning@* 0-1{{stack nearly exhausted}}
 // expected-note@* 0+{{}}
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67117: [clangd] Split Preamble.h out of ClangdUnit.h. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous, 
MaskRay, javed.absar, ilya-biryukov, mgorny.
Herald added a project: clang.

Add comment describing use of preamble in clangd.
Remove deps on ClangdUnit.h where possible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67117

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -12,6 +12,7 @@
 #include "Diagnostics.h"
 #include "Matchers.h"
 #include "Path.h"
+#include "Preamble.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
 #include "Threading.h"
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 
-#include "ClangdUnit.h"
+#include "Compiler.h"
 #include "Diagnostics.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
@@ -24,6 +24,8 @@
 
 namespace clang {
 namespace clangd {
+class ParsedAST;
+struct PreambleData;
 
 /// Returns a number of a default async threads to use for TUScheduler.
 /// Returned value is always >= 1 (i.e. will not cause requests to be processed
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -43,10 +43,12 @@
 
 #include "TUScheduler.h"
 #include "Cancellation.h"
+#include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
+#include "Preamble.h"
 #include "Trace.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/CompilerInvocation.h"
Index: clang-tools-extra/clangd/Preamble.h
===
--- /dev/null
+++ clang-tools-extra/clangd/Preamble.h
@@ -0,0 +1,88 @@
+//===--- Preamble.h - Reusing expensive parts of the AST -*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// The vast majority of code in a typical translation unit is in the headers
+// included at the top of the file.
+//
+// The preamble optimization says that we can parse this code once, and reuse
+// the result multiple times. The preamble is invalidated by changes to the
+// code in the preamble region, to the compile command, or to files on disk.
+//
+// This is the most important optimization in clangd: it allows operations like
+// code-completion to have sub-second latency. It is supported by the
+// PrecompiledPreamble functionality in clang, which wraps the techniques used
+// by PCH files, modules etc into a convenient interface.
+//
+//===--===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H
+
+#include "Compiler.h"
+#include "Diagnostics.h"
+#include "FS.h"
+#include "Headers.h"
+#include "index/CanonicalIncludes.h"
+#include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Tooling/CompilationDatabase.h"
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// The parsed preamble and associated data.
+///
+/// As we must avoid re-parsing the preamble, any information that can only
+/// be obtained during parsing must be eagerly captured and stored here.
+struct PreambleData {
+  PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+   IncludeStructure Includes,
+   std::vector MainFileMacros,
+   std::unique_ptr StatCache,
+   CanonicalIncludes CanonIncludes);
+
+  tooling::CompileCommand CompileCommand;
+  PrecompiledPreamble Preamble;
+  std::vector Diags;
+  // Processes like code completions and go-to-definitions will need #include
+  // information, and their compile action skips preamble range.
+  IncludeStructur

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

[Kristóf Umann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370798: [analyzer] NonNullParamChecker and CStringChecker 
parameter number in checker… (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66333?vs=215754&id=218492#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66333/new/

https://reviews.llvm.org/D66333

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  cfe/trunk/test/Analysis/Inputs/expected-plists/plist-output.m.plist
  cfe/trunk/test/Analysis/misc-ps-region-store.m
  cfe/trunk/test/Analysis/null-deref-ps.c

Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -36,7 +36,9 @@
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
   std::unique_ptr
-  genReportNullAttrNonNull(const ExplodedNode *ErrorN, const Expr *ArgE) const;
+  genReportNullAttrNonNull(const ExplodedNode *ErrorN,
+   const Expr *ArgE,
+   unsigned IdxOfArg) const;
   std::unique_ptr
   genReportReferenceToNullPointer(const ExplodedNode *ErrorN,
   const Expr *ArgE) const;
@@ -143,7 +145,7 @@
 
 std::unique_ptr R;
 if (haveAttrNonNull)
-  R = genReportNullAttrNonNull(errorNode, ArgE);
+  R = genReportNullAttrNonNull(errorNode, ArgE, idx + 1);
 else if (haveRefTypeParam)
   R = genReportReferenceToNullPointer(errorNode, ArgE);
 
@@ -179,7 +181,8 @@
 
 std::unique_ptr
 NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode,
-  const Expr *ArgE) const {
+  const Expr *ArgE,
+  unsigned IdxOfArg) const {
   // Lazily allocate the BugType object if it hasn't already been
   // created. Ownership is transferred to the BugReporter object once
   // the BugReport is passed to 'EmitWarning'.
@@ -187,9 +190,13 @@
 BTAttrNonNull.reset(new BugType(
 this, "Argument with 'nonnull' attribute passed null", "API"));
 
-  auto R = std::make_unique(
-  *BTAttrNonNull,
-  "Null pointer passed as an argument to a 'nonnull' parameter", ErrorNode);
+  llvm::SmallString<256> SBuf;
+  llvm::raw_svector_ostream OS(SBuf);
+  OS << "Null pointer passed to "
+ << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg)
+ << " parameter expecting 'nonnull'";
+
+  auto R = llvm::make_unique(*BTAttrNonNull, SBuf, ErrorNode);
   if (ArgE)
 bugreporter::trackExpressionValue(ErrorNode, ArgE, *R);
 
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -198,7 +198,8 @@
   ProgramStateRef checkNonNull(CheckerContext &C,
ProgramStateRef state,
const Expr *S,
-   SVal l) const;
+   SVal l,
+   unsigned IdxOfArg) const;
   ProgramStateRef CheckLocation(CheckerContext &C,
 ProgramStateRef state,
 const Expr *S,
@@ -277,7 +278,8 @@
 
 ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
 ProgramStateRef state,
-const Expr *S, SVal l) const {
+const Expr *S, SVal l,
+unsigned IdxOfArg) const {
   // If a previous check has failed, propagate the failure.
   if (!state)
 return nullptr;
@@ -288,11 +290,13 @@
   if (stateNull && !stateNonNull) {
 if (Filter.CheckCStringNullArg) {
   SmallString<80> buf;
-  llvm::raw_svector_ostream os(buf);
+  llvm::raw_svector_ostream OS(buf);
   assert(CurrentFunctionDescription);
-  os << "Null pointer argument in call to " << CurrentFunctionDescription;
+  OS << "Null pointer argument in call to " << CurrentFunctionDescription
+ << ' ' << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg)
+ << " parameter";
 
-  emitNullArgBug(C, stateNull, S, os.str());
+  emitNullArgBug(C, stateNull, S, OS.str());
 }
 return nullptr;
   }
@@ -384,7 +388,7 @@
 
   // Check that the first buffer is non-null.
   SVal BufVal

r370798 - [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Sep  3 10:57:01 2019
New Revision: 370798

URL: http://llvm.org/viewvc/llvm-project?rev=370798&view=rev
Log:
[analyzer] NonNullParamChecker and CStringChecker parameter number in checker 
message

There are some functions which can't be given a null pointer as parameter either
because it has a nonnull attribute or it is declared to have undefined behavior
(e.g. strcmp()). Sometimes it is hard to determine from the checker message
which parameter is null at the invocation, so now this information is included
in the message.

This commit fixes https://bugs.llvm.org/show_bug.cgi?id=39358

Reviewed By: NoQ, Szelethus, whisperity

Patch by Tibor Brunner!

Differential Revision: https://reviews.llvm.org/D66333

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
cfe/trunk/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-output.m.plist
cfe/trunk/test/Analysis/misc-ps-region-store.m
cfe/trunk/test/Analysis/null-deref-ps.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=370798&r1=370797&r2=370798&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Tue Sep  3 
10:57:01 2019
@@ -198,7 +198,8 @@ public:
   ProgramStateRef checkNonNull(CheckerContext &C,
ProgramStateRef state,
const Expr *S,
-   SVal l) const;
+   SVal l,
+   unsigned IdxOfArg) const;
   ProgramStateRef CheckLocation(CheckerContext &C,
 ProgramStateRef state,
 const Expr *S,
@@ -277,7 +278,8 @@ CStringChecker::assumeZero(CheckerContex
 
 ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
 ProgramStateRef state,
-const Expr *S, SVal l) const {
+const Expr *S, SVal l,
+unsigned IdxOfArg) const {
   // If a previous check has failed, propagate the failure.
   if (!state)
 return nullptr;
@@ -288,11 +290,13 @@ ProgramStateRef CStringChecker::checkNon
   if (stateNull && !stateNonNull) {
 if (Filter.CheckCStringNullArg) {
   SmallString<80> buf;
-  llvm::raw_svector_ostream os(buf);
+  llvm::raw_svector_ostream OS(buf);
   assert(CurrentFunctionDescription);
-  os << "Null pointer argument in call to " << CurrentFunctionDescription;
+  OS << "Null pointer argument in call to " << CurrentFunctionDescription
+ << ' ' << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg)
+ << " parameter";
 
-  emitNullArgBug(C, stateNull, S, os.str());
+  emitNullArgBug(C, stateNull, S, OS.str());
 }
 return nullptr;
   }
@@ -384,7 +388,7 @@ ProgramStateRef CStringChecker::CheckBuf
 
   // Check that the first buffer is non-null.
   SVal BufVal = C.getSVal(FirstBuf);
-  state = checkNonNull(C, state, FirstBuf, BufVal);
+  state = checkNonNull(C, state, FirstBuf, BufVal, 1);
   if (!state)
 return nullptr;
 
@@ -424,7 +428,7 @@ ProgramStateRef CStringChecker::CheckBuf
   // If there's a second buffer, check it as well.
   if (SecondBuf) {
 BufVal = state->getSVal(SecondBuf, LCtx);
-state = checkNonNull(C, state, SecondBuf, BufVal);
+state = checkNonNull(C, state, SecondBuf, BufVal, 2);
 if (!state)
   return nullptr;
 
@@ -1163,7 +1167,7 @@ void CStringChecker::evalCopyCommon(Chec
 
 // Ensure the destination is not null. If it is NULL there will be a
 // NULL pointer dereference.
-state = checkNonNull(C, state, Dest, destVal);
+state = checkNonNull(C, state, Dest, destVal, 1);
 if (!state)
   return;
 
@@ -1172,7 +1176,7 @@ void CStringChecker::evalCopyCommon(Chec
 
 // Ensure the source is not null. If it is NULL there will be a
 // NULL pointer dereference.
-state = checkNonNull(C, state, Source, srcVal);
+state = checkNonNull(C, state, Source, srcVal, 2);
 if (!state)
   return;
 
@@ -1383,7 +1387,7 @@ void CStringChecker::evalstrLengthCommon
   const Expr *Arg = CE->getArg(0);
   SVal ArgVal = state->getSVal(Arg, LCtx);
 
-  state = checkNonNull(C, state, Arg, ArgVal);
+  state = checkNonNull(C, state, Arg, ArgVal, 1);
 
   if (!state)
 return;
@@ -1541,14 +1545,14 @@ void CStringChecker::evalStrcpyCommon(Ch
   const Expr *Dst = CE->getArg(0);
   SVal DstVal = state->getSVal(Dst, LCtx);
 
-  state = checkNonNull(C, state, Dst, DstVal);
+  state = 

r370795 - [www] Mark items complete in Clang 9 as 'Clang 9' rather than 'SVN'.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Sep  3 10:49:51 2019
New Revision: 370795

URL: http://llvm.org/viewvc/llvm-project?rev=370795&view=rev
Log:
[www] Mark items complete in Clang 9 as 'Clang 9' rather than 'SVN'.

Don't turn the boxes green yet, since Clang 9 hasn't been released.

Modified:
cfe/trunk/www/cxx_dr_status.html
cfe/trunk/www/cxx_status.html
cfe/trunk/www/make_cxx_dr_status

Modified: cfe/trunk/www/cxx_dr_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_dr_status.html?rev=370795&r1=370794&r2=370795&view=diff
==
--- cfe/trunk/www/cxx_dr_status.html (original)
+++ cfe/trunk/www/cxx_dr_status.html Tue Sep  3 10:49:51 2019
@@ -9955,19 +9955,19 @@ and POD class
 http://wg21.link/cwg1690";>1690
 C++14
 Associated namespace for local type
-SVN
+Clang 9
   
   
 http://wg21.link/cwg1691";>1691
 C++14
 Argument-dependent lookup and opaque enumerations
-SVN
+Clang 9
   
   
 http://wg21.link/cwg1692";>1692
 C++14
 Associated namespaces of doubly-nested classes
-SVN
+Clang 9
   
   
 http://wg21.link/cwg1693";>1693
@@ -10147,7 +10147,7 @@ and POD class
 http://wg21.link/cwg1722";>1722
 CD4
 Should lambda to function pointer conversion function be 
noexcept?
-SVN
+Clang 9
   
   
 http://wg21.link/cwg1723";>1723
@@ -10483,7 +10483,7 @@ and POD class
 http://wg21.link/cwg1778";>1778
 C++14
 exception-specification in explicitly-defaulted functions
-SVN
+Clang 9
   
   
 http://wg21.link/cwg1779";>1779
@@ -11047,7 +11047,7 @@ and POD class
 http://wg21.link/cwg1872";>1872
 CD4
 Instantiations of constexpr templates that cannot appear in 
constant expressions
-SVN
+Clang 9
   
   
 http://wg21.link/cwg1873";>1873
@@ -12655,7 +12655,7 @@ and POD class
 http://wg21.link/cwg2140";>2140
 CD4
 Lvalue-to-rvalue conversion of std::nullptr_t
-SVN
+Clang 9
   
   
 http://wg21.link/cwg2141";>2141
@@ -12835,7 +12835,7 @@ and POD class
 http://wg21.link/cwg2170";>2170
 DR
 Unclear definition of odr-use for arrays
-SVN
+Clang 9
   
   
 http://wg21.link/cwg2171";>2171
@@ -13567,7 +13567,7 @@ and POD class
 http://wg21.link/cwg2292";>2292
 DRWP
 simple-template-id is ambiguous between class-name and 
type-name
-SVN
+Clang 9
   
   
 http://wg21.link/cwg2293";>2293
@@ -13933,7 +13933,7 @@ and POD class
 http://wg21.link/cwg2353";>2353
 DR
 Potential results of a member access expression for a static data 
member
-SVN
+Clang 9
   
   
 http://wg21.link/cwg2354";>2354
@@ -14131,13 +14131,13 @@ and POD class
 http://wg21.link/cwg2386";>2386
 DR
 tuple_size requirements for structured binding
-Unknown
+Clang 9
   
   
 http://wg21.link/cwg2387";>2387
 DR
 Linkage of const-qualified variable template
-SVN
+Clang 9
   
   
 http://wg21.link/cwg2388";>2388

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=370795&r1=370794&r2=370795&view=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Tue Sep  3 10:49:51 2019
@@ -114,7 +114,7 @@ with http://libcxx.llvm.org/";>l
 

 http://wg21.link/p1009r2";>P1009R2 (DR)
-SVN
+Clang 9
   
 
   Static assertions
@@ -289,7 +289,7 @@ with http://libcxx.llvm.org/";>l
 

 http://wg21.link/p1286r2";>P1286R2 (DR)
-SVN
+Clang 9
   
 
   Deleted functions
@@ -664,7 +664,7 @@ version 3.7.
 

 http://wg21.link/p1771r1";>P1771R1 (DR)
-SVN
+Clang 9
   
 
   [[maybe_unused]] attribute
@@ -866,17 +866,17 @@ as the draft C++2a standard evolves.
 

 http://wg21.link/p1042r1";>P1042R1
-SVN
+Clang 9
   
 
   Designated initializers
   http://wg21.link/p0329r4";>P0329R4
-  SVN (Clang 10)
+  SVN
 
 
   template-parameter-list for generic lambdas
   http://wg21.link/p0428r2";>P0428R2
-  SVN
+  Clang 9
 
 
   Concepts
@@ -910,7 +910,7 @@ as the draft C++2a standard evolves.
 
   ADL and function templates that are not visible
   http://wg21.link/p0846r0";>P0846R0
-  SVN
+  Clang 9
 
 
   const mismatch with defaulted copy constructor
@@ -957,7 +957,7 @@ as the draft C++2a standard evolves.
 
   [[no_unique_address]] attribute
   http://wg21.link/p0840r2";>P0840R2
-  SVN
+  Clang 9
 
 
   [[likely]] and [[unlikely]] attributes
@@ -972,7 +972,7 @@ as the draft C++2a standard evolves.
 
   Pack expansion in lambda init-capture
   http://wg21.link/p0780r2";>P0780R2

[PATCH] D67113: ICK_Function_Conversion and ICK_Qualification are third kind conversions

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Not sure if this has any effect, but it was inconsistent before.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67113

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5339,7 +5339,6 @@
   // conversions are fine.
   switch (SCS.Second) {
   case ICK_Identity:
-  case ICK_Function_Conversion:
   case ICK_Integral_Promotion:
   case ICK_Integral_Conversion: // Narrowing conversions are checked elsewhere.
   case ICK_Zero_Queue_Conversion:
@@ -5385,6 +5384,7 @@
   case ICK_Function_To_Pointer:
 llvm_unreachable("found a first conversion kind in Second");
 
+  case ICK_Function_Conversion:
   case ICK_Qualification:
 llvm_unreachable("found a third conversion kind in Second");
 
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4252,12 +4252,14 @@
   case ICK_Lvalue_To_Rvalue:
   case ICK_Array_To_Pointer:
   case ICK_Function_To_Pointer:
-  case ICK_Function_Conversion:
-  case ICK_Qualification:
   case ICK_Num_Conversion_Kinds:
   case ICK_C_Only_Conversion:
   case ICK_Incompatible_Pointer_Conversion:
 llvm_unreachable("Improper second standard conversion");
+
+  case ICK_Function_Conversion:
+  case ICK_Qualification:
+llvm_unreachable("Improper third standard conversion");
   }
 
   switch (SCS.Third) {


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5339,7 +5339,6 @@
   // conversions are fine.
   switch (SCS.Second) {
   case ICK_Identity:
-  case ICK_Function_Conversion:
   case ICK_Integral_Promotion:
   case ICK_Integral_Conversion: // Narrowing conversions are checked elsewhere.
   case ICK_Zero_Queue_Conversion:
@@ -5385,6 +5384,7 @@
   case ICK_Function_To_Pointer:
 llvm_unreachable("found a first conversion kind in Second");
 
+  case ICK_Function_Conversion:
   case ICK_Qualification:
 llvm_unreachable("found a third conversion kind in Second");
 
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4252,12 +4252,14 @@
   case ICK_Lvalue_To_Rvalue:
   case ICK_Array_To_Pointer:
   case ICK_Function_To_Pointer:
-  case ICK_Function_Conversion:
-  case ICK_Qualification:
   case ICK_Num_Conversion_Kinds:
   case ICK_C_Only_Conversion:
   case ICK_Incompatible_Pointer_Conversion:
 llvm_unreachable("Improper second standard conversion");
+
+  case ICK_Function_Conversion:
+  case ICK_Qualification:
+llvm_unreachable("Improper third standard conversion");
   }
 
   switch (SCS.Third) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66748: [PowerPC][Altivec][Clang] Check compile-time constant for vec_dst*

[Jinsong Ji via Phabricator via cfe-commits]

jsji updated this revision to Diff 218485.
jsji added a comment.

Add range check and tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66748/new/

https://reviews.llvm.org/D66748

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-error.c


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,15 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument 
to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 
5 is outside the valid range [0, 1]}}
 }
+
+
+void testDST(int index) {
+  vec_dst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dst' must be a constant integer}}
+  vec_dst(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstt' must be a constant integer}}
+  vec_dstt(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstst' must be a constant integer}}
+  vec_dstst(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+  vec_dststt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dststt' must be a constant integer}}
+  vec_dststt(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3227,6 +3227,11 @@
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  case PPC::BI__builtin_altivec_dst:
+  case PPC::BI__builtin_altivec_dstt:
+  case PPC::BI__builtin_altivec_dstst:
+  case PPC::BI__builtin_altivec_dststt:
+return SemaBuiltinConstantArgRange(TheCall, 2, 0, 3);
   case PPC::BI__builtin_vsx_xxpermdi:
   case PPC::BI__builtin_vsx_xxsldwi:
 return SemaBuiltinVSX(TheCall);
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -57,10 +57,10 @@
 
 BUILTIN(__builtin_altivec_dss, "vUi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
-BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dststt, "vvC*iUi", "")
+BUILTIN(__builtin_altivec_dst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstt, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dststt, "vvC*iUIi", "")
 
 BUILTIN(__builtin_altivec_vexptefp, "V4fV4f", "")
 


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,15 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 5 is outside the valid range [0, 1]}}
 }
+
+
+void testDST(int index) {
+  vec_dst(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dst' must be a constant integer}}
+  vec_dst(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+  vec_dstt(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dstt' must be a constant integer}}
+  vec_dstt(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+  vec_dstst(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dstst' must be a constant integer}}
+  vec_dstst(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+  vec_dststt(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dststt' must be a constant integer}}
+  vec_dststt(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3227,6 +3227,11 @@
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  ca

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.

If anyone shares my feeling that the boolean output parameters of 
`CompareReferenceRelationship` should rather move to the return value, I would 
be happy to do that.




Comment at: clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp:5
+// CHECK: VarDecl {{.*}} fp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void () noexcept' lvalue Function {{.*}} 
'f' 'void () noexcept'

This is new.



Comment at: clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp:15
+// CHECK: VarDecl {{.*}} xp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void () noexcept':'void () noexcept' 
lvalue 

This is also new.


When initializing a reference to a non-noexcept function from a noexcept
function we didn't add an implicit cast node, which confused a type-
checking assertion in CodeGen. The same conversion is used for function
pointer conversion already.

Fixes PR43154.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67112

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp

Index: clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++17 -ast-dump %s | FileCheck %s
+
+void f() noexcept;
+// CHECK: VarDecl {{.*}} fp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void () noexcept' lvalue Function {{.*}} 'f' 'void () noexcept'
+void (&fp)() = f;
+
+struct X {
+  typedef void (&fp)() noexcept;
+  operator fp();
+} x;
+
+// CHECK: VarDecl {{.*}} xp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void () noexcept':'void () noexcept' lvalue 
+void (&xp)() = x;
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -4296,12 +4296,10 @@
 /// reference (C++ [dcl.ref.init]p4). Neither type can be a reference
 /// type, and the first type (T1) is the pointee type of the reference
 /// type being initialized.
-Sema::ReferenceCompareResult
-Sema::CompareReferenceRelationship(SourceLocation Loc,
-   QualType OrigT1, QualType OrigT2,
-   bool &DerivedToBase,
-   bool &ObjCConversion,
-   bool &ObjCLifetimeConversion) {
+Sema::ReferenceCompareResult Sema::CompareReferenceRelationship(
+SourceLocation Loc, QualType OrigT1, QualType OrigT2, bool &DerivedToBase,
+bool &FuncRefConversion, bool &ObjCConversion,
+bool &ObjCLifetimeConversion) {
   assert(!OrigT1->isReferenceType() &&
 "T1 must be the pointee type of the reference type");
   assert(!OrigT2->isReferenceType() && "T2 cannot be a reference type");
@@ -4317,6 +4315,7 @@
   //   reference-related to "cv2 T2" if T1 is the same type as T2, or
   //   T1 is a base class of T2.
   DerivedToBase = false;
+  FuncRefConversion = false;
   ObjCConversion = false;
   ObjCLifetimeConversion = false;
   QualType ConvertedT2;
@@ -4331,15 +4330,16 @@
Context.canBindObjCObjectType(UnqualT1, UnqualT2))
 ObjCConversion = true;
   else if (UnqualT2->isFunctionType() &&
-   IsFunctionConversion(UnqualT2, UnqualT1, ConvertedT2))
+   IsFunctionConversion(UnqualT2, UnqualT1, ConvertedT2)) {
 // C++1z [dcl.init.ref]p4:
 //   cv1 T1" is reference-compatible with "cv2 T2" if [...] T2 is "noexcept
 //   function" and T1 is "function"
 //
 // We extend this to also apply to 'noreturn', so allow any function
 // conversion between function types.
+FuncRefConversion = true;
 return Ref_Compatible;
-  else
+  } else
 return Ref_Incompatible;
 
   // At this point, we know that T1 and T2 are reference-related (at
@@ -4418,6 +4418,7 @@
 
 if (AllowRvalues) {
   bool DerivedToBase = false;
+  bool FuncPtrConversion = false;
   bool ObjCConversion = false;
   bool ObjCLifetimeConversion = false;
 
@@ -4432,12 +4433,13 @@
 
   if (!ConvTemplate &&
   S.CompareReferenceRelationship(
-

[PATCH] D66665: [CUDA] Use activemask.b32 instruction to implement __activemask w/ CUDA-9.2+

[Artem Belevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370792: [CUDA] Use activemask.b32 instruction to implement 
__activemask w/ CUDA-9.2+ (authored by tra, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D5?vs=216886&id=218486#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D5/new/

https://reviews.llvm.org/D5

Files:
  cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h


Index: cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
===
--- cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
+++ cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
@@ -211,7 +211,15 @@
   return __nvvm_vote_ballot_sync(mask, pred);
 }
 
-inline __device__ unsigned int __activemask() { return __nvvm_vote_ballot(1); }
+inline __device__ unsigned int __activemask() {
+#if CUDA_VERSION < 9020
+  return __nvvm_vote_ballot(1);
+#else
+  unsigned int mask;
+  asm volatile("activemask.b32 %0;" : "=r"(mask));
+  return mask;
+#endif
+}
 
 inline __device__ unsigned int __fns(unsigned mask, unsigned base, int offset) 
{
   return __nvvm_fns(mask, base, offset);


Index: cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
===
--- cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
+++ cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
@@ -211,7 +211,15 @@
   return __nvvm_vote_ballot_sync(mask, pred);
 }
 
-inline __device__ unsigned int __activemask() { return __nvvm_vote_ballot(1); }
+inline __device__ unsigned int __activemask() {
+#if CUDA_VERSION < 9020
+  return __nvvm_vote_ballot(1);
+#else
+  unsigned int mask;
+  asm volatile("activemask.b32 %0;" : "=r"(mask));
+  return mask;
+#endif
+}
 
 inline __device__ unsigned int __fns(unsigned mask, unsigned base, int offset) {
   return __nvvm_fns(mask, base, offset);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.

If anyone shares my feeling that the boolean output parameters of 
`CompareReferenceRelationship` should rather move to the return value, I would 
be happy to do that.




Comment at: clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp:5
+// CHECK: VarDecl {{.*}} fp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void () noexcept' lvalue Function {{.*}} 
'f' 'void () noexcept'

This is new.



Comment at: clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp:15
+// CHECK: VarDecl {{.*}} xp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void () noexcept':'void () noexcept' 
lvalue 

This is also new.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67112/new/

https://reviews.llvm.org/D67112



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r370792 - [CUDA] Use activemask.b32 instruction to implement __activemask w/ CUDA-9.2+

[Artem Belevich via cfe-commits]

Author: tra
Date: Tue Sep  3 10:31:58 2019
New Revision: 370792

URL: http://llvm.org/viewvc/llvm-project?rev=370792&view=rev
Log:
[CUDA] Use activemask.b32 instruction to implement __activemask w/ CUDA-9.2+

vote.ballot instruction is gone in recent CUDA versions and
vote.sync.ballot can not be used because it needs a thread mask parameter.
Fortunately PTX 6.2 (introduced with CUDA-9.2) provides activemask.b32
instruction for this.

Differential Revision: https://reviews.llvm.org/D5

Modified:
cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h

Modified: cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h?rev=370792&r1=370791&r2=370792&view=diff
==
--- cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h (original)
+++ cfe/trunk/lib/Headers/__clang_cuda_intrinsics.h Tue Sep  3 10:31:58 2019
@@ -211,7 +211,15 @@ inline __device__ unsigned int __ballot_
   return __nvvm_vote_ballot_sync(mask, pred);
 }
 
-inline __device__ unsigned int __activemask() { return __nvvm_vote_ballot(1); }
+inline __device__ unsigned int __activemask() {
+#if CUDA_VERSION < 9020
+  return __nvvm_vote_ballot(1);
+#else
+  unsigned int mask;
+  asm volatile("activemask.b32 %0;" : "=r"(mask));
+  return mask;
+#endif
+}
 
 inline __device__ unsigned int __fns(unsigned mask, unsigned base, int offset) 
{
   return __nvvm_fns(mask, base, offset);


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

This change broke on NetBSD.

http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22003/steps/run%20unit%20tests/logs/FAIL%3A%20Clang%3A%3Astack-exhaustion.cpp

   (view as text)
  
   TEST 'Clang :: SemaTemplate/stack-exhaustion.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   /home/motus/netbsd8/netbsd8/build/bin/clang -cc1 
-internal-isystem /data/motus/netbsd8/netbsd8/build/lib/clang/10.0.0/include 
-nostdsysteminc -verify 
/data/motus/netbsd8/netbsd8/llvm/tools/clang/test/SemaTemplate/stack-exhaustion.cpp
  --
  Exit Code: 139
  
  Command Output (stderr):
  --
  + : 'RUN: at line 1'
  + /home/motus/netbsd8/netbsd8/build/bin/clang -cc1 -internal-isystem 
/data/motus/netbsd8/netbsd8/build/lib/clang/10.0.0/include -nostdsysteminc 
-verify 
/data/motus/netbsd8/netbsd8/llvm/tools/clang/test/SemaTemplate/stack-exhaustion.cpp
  Stack dump:
  0.Program arguments: /home/motus/netbsd8/netbsd8/build/bin/clang -cc1 
-internal-isystem /data/motus/netbsd8/netbsd8/build/lib/clang/10.0.0/include 
-nostdsysteminc -verify 
/data/motus/netbsd8/netbsd8/llvm/tools/clang/test/SemaTemplate/stack-exhaustion.cpp
 
  1.
/data/motus/netbsd8/netbsd8/llvm/tools/clang/test/SemaTemplate/stack-exhaustion.cpp:9:10:
 current parser token ';'
  #0 0x0242038c llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(/home/motus/netbsd8/netbsd8/build/bin/clang+0x242038c)
  #1 0x0241e5ee llvm::sys::RunSignalHandlers() 
(/home/motus/netbsd8/netbsd8/build/bin/clang+0x241e5ee)
  #2 0x0241e6ee SignalHandler(int) 
(/home/motus/netbsd8/netbsd8/build/bin/clang+0x241e6ee)
  
/data/motus/netbsd8/netbsd8/build/tools/clang/test/SemaTemplate/Output/stack-exhaustion.cpp.script:
 line 1:  7058 Segmentation fault  (core dumped) 
/home/motus/netbsd8/netbsd8/build/bin/clang -cc1 -internal-isystem 
/data/motus/netbsd8/netbsd8/build/lib/clang/10.0.0/include -nostdsysteminc 
-verify 
/data/motus/netbsd8/netbsd8/llvm/tools/clang/test/SemaTemplate/stack-exhaustion.cpp
  
  --
  
  

Please fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66361/new/

https://reviews.llvm.org/D66361



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[David Gatwood via Phabricator via cfe-commits]

dgatwood marked 17 inline comments as done.
dgatwood added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57
+  }
+  std::string method_name = method_declaration->getNameAsString();
+  auto owning_objc_class_interface = method_declaration->getClassInterface();

aaron.ballman wrote:
> This should use `getName()` to get a `StringRef` to avoid the copy.
That's actually what I originally tried, but that method won't work here, 
unless I'm missing something.  The getName() method crashes with a message 
saying that "Name is not a simple identifier".



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:19
+
+You should set the clang option WhitelistedPrefixes to a semicolon-delimited
+lits of class prefixes within your project if you want to be able to create

Eugene.Zelenko wrote:
> See other checks documentation for proper option section style.
> 
> clang -> :program:`clang-tidy` and enclose WhitelistedPrefixes in single 
> back-ticks.
Removed the text in question, because it duplicates the Options info.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65917/new/

https://reviews.llvm.org/D65917



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66699: [PowerPC][Altivec] Fix constant argument for vec_dss

[Jinsong Ji via Phabricator via cfe-commits]

jsji marked 2 inline comments as done.
jsji added inline comments.



Comment at: clang/test/CodeGen/builtins-ppc-error.c:78
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
+

wuzish wrote:
> It would be better if add range constraint check at 
> Sema::CheckPPCBuiltinFunctionCall
Good point, Done. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66699/new/

https://reviews.llvm.org/D66699



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66699: [PowerPC][Altivec] Fix constant argument for vec_dss

[Jinsong Ji via Phabricator via cfe-commits]

jsji updated this revision to Diff 218479.
jsji added a comment.

Add range check and test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66699/new/

https://reviews.llvm.org/D66699

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/altivec-dss.c
  clang/test/CodeGen/builtins-ppc-error.c


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,8 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument 
to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 
5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
+  vec_dss(5); //expected-error {{argument value 5 is outside the valid range 
[0, 3]}}
+}
Index: clang/test/CodeGen/altivec-dss.c
===
--- /dev/null
+++ clang/test/CodeGen/altivec-dss.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature 
+altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3220,6 +3220,8 @@
   case PPC::BI__builtin_altivec_crypto_vshasigmad:
 return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 15);
+  case PPC::BI__builtin_altivec_dss:
+return SemaBuiltinConstantArgRange(TheCall, 0, 0, 3);
   case PPC::BI__builtin_tbegin:
   case PPC::BI__builtin_tend: i = 0; l = 0; u = 1; break;
   case PPC::BI__builtin_tsr: i = 0; l = 0; u = 7; break;
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -3286,9 +3286,7 @@
 
 /* vec_dss */
 
-static __inline__ void __attribute__((__always_inline__)) vec_dss(int __a) {
-  __builtin_altivec_dss(__a);
-}
+#define vec_dss __builtin_altivec_dss
 
 /* vec_dssall */
 
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -55,7 +55,7 @@
 BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fIi", "")
 BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fIi", "")
 
-BUILTIN(__builtin_altivec_dss, "vUi", "")
+BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
 BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
 BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,8 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must be a constant integer}}
+  vec_dss(5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+}
Index: clang/test/CodeGen/altivec-dss.c
===
--- /dev/null
+++ clang/test/CodeGen/altivec-dss.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature +altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3220,6 +3220,8 @@
   case PPC::BI__builtin_altivec_crypto_vshasigmad:
 return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 15);
+  case PPC::BI__builtin_altivec_dss:
+return SemaBuiltinConstantArgRange(TheCall, 0, 0, 3);
   case PPC::BI__builtin_tbegin:
   case PPC::BI__builtin_tend: i = 0; l = 0; u = 1; break;
   case PPC::BI__builtin_tsr: i = 0; l = 0; u = 7; break;
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers