r333413 - Fix emission of phony dependency targets when adding extra deps

2018-05-29 Thread David Stenberg via cfe-commits 
Author: dstenb
Date: Tue May 29 06:07:58 2018
New Revision: 333413

URL: http://llvm.org/viewvc/llvm-project?rev=333413&view=rev
Log:
Fix emission of phony dependency targets when adding extra deps

Summary:
This commit fixes a bug where passing extra dependency entries
(using -fdepfile-entry) would result in -MP incorrectly emitting
a phony target for the input file, and no phony target for the
first extra dependency.

The extra dependencies are added first to the filename vector in
DFGImpl. That clashed with the emission of the phony targets, as
the code assumed that the first index always correspond to the
input file.

Reviewers: rsmith, pcc, krasin, bruno, vsapsai

Reviewed By: vsapsai

Subscribers: vsapsai, bruno, cfe-commits

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

Added:
cfe/trunk/test/Frontend/dependency-gen-extradeps-phony.c
Modified:
cfe/trunk/lib/Frontend/DependencyFile.cpp

Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=333413&r1=333412&r2=333413&view=diff
==
--- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
+++ cfe/trunk/lib/Frontend/DependencyFile.cpp Tue May 29 06:07:58 2018
@@ -163,6 +163,7 @@ class DFGImpl : public PPCallbacks {
   bool SeenMissingHeader;
   bool IncludeModuleFiles;
   DependencyOutputFormat OutputFormat;
+  unsigned InputFileIndex;
 
 private:
   bool FileMatchesDepCriteria(const char *Filename,
@@ -177,9 +178,11 @@ public:
   AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
   SeenMissingHeader(false),
   IncludeModuleFiles(Opts.IncludeModuleFiles),
-  OutputFormat(Opts.OutputFormat) {
+  OutputFormat(Opts.OutputFormat),
+  InputFileIndex(0) {
 for (const auto &ExtraDep : Opts.ExtraDeps) {
-  AddFilename(ExtraDep);
+  if (AddFilename(ExtraDep))
+++InputFileIndex;
 }
   }
 
@@ -201,7 +204,7 @@ public:
 OutputDependencyFile();
   }
 
-  void AddFilename(StringRef Filename);
+  bool AddFilename(StringRef Filename);
   bool includeSystemHeaders() const { return IncludeSystemHeaders; }
   bool includeModuleFiles() const { return IncludeModuleFiles; }
 };
@@ -325,9 +328,12 @@ void DFGImpl::InclusionDirective(SourceL
   }
 }
 
-void DFGImpl::AddFilename(StringRef Filename) {
-  if (FilesSet.insert(Filename).second)
+bool DFGImpl::AddFilename(StringRef Filename) {
+  if (FilesSet.insert(Filename).second) {
 Files.push_back(Filename);
+return true;
+  }
+  return false;
 }
 
 /// Print the filename, with escaping or quoting that accommodates the three
@@ -463,8 +469,10 @@ void DFGImpl::OutputDependencyFile() {
 
   // Create phony targets if requested.
   if (PhonyTarget && !Files.empty()) {
-// Skip the first entry, this is always the input file itself.
-for (auto I = Files.begin() + 1, E = Files.end(); I != E; ++I) {
+unsigned Index = 0;
+for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
+  if (Index++ == InputFileIndex)
+continue;
   OS << '\n';
   PrintFilename(OS, *I, OutputFormat);
   OS << ":\n";

Added: cfe/trunk/test/Frontend/dependency-gen-extradeps-phony.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/dependency-gen-extradeps-phony.c?rev=333413&view=auto
==
--- cfe/trunk/test/Frontend/dependency-gen-extradeps-phony.c (added)
+++ cfe/trunk/test/Frontend/dependency-gen-extradeps-phony.c Tue May 29 
06:07:58 2018
@@ -0,0 +1,10 @@
+// RUN: %clang -MM -MP -Xclang -fdepfile-entry=1.extra -Xclang 
-fdepfile-entry=2.extra -Xclang -fdepfile-entry=2.extra %s | \
+// RUN: FileCheck %s --implicit-check-not=.c:
+//
+// Verify that phony targets are only created for the extra dependency files,
+// and not the input file.
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra \
+// CHECK-NEXT: dependency-gen-extradeps-phony.c
+// CHECK: 1.extra:
+// CHECK: 2.extra:


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

r333637 - [Driver] Clean up tmp files when deleting Compilation objects

2018-05-31 Thread David Stenberg via cfe-commits 
Author: dstenb
Date: Thu May 31 02:05:22 2018
New Revision: 333637

URL: http://llvm.org/viewvc/llvm-project?rev=333637&view=rev
Log:
[Driver] Clean up tmp files when deleting Compilation objects

Summary:
In rL327851 the createUniqueFile() and createTemporaryFile()
variants that do not return the file descriptors were changed to
create empty files, rather than only check if the paths are free.
This change was done in order to make the functions race-free.

That change led to clang-tidy (and possibly other tools) leaving
behind temporary assembly files, of the form placeholder-*, when
using a target that does not support the internal assembler.

The temporary files are created when building the Compilation
object in stripPositionalArgs(), as a part of creating the
compilation database for the arguments after the double-dash. The
files are created by Driver::GetNamedOutputPath().

Fix this issue by cleaning out temporary files at the deletion of
Compilation objects.

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

Reviewers: klimek, sepavloff, arphaman, aaron.ballman, john.brawn, mehdi_amini, 
sammccall, bkramer, alexfh, JDevlieghere

Reviewed By: aaron.ballman, JDevlieghere

Subscribers: erichkeane, lebedev.ri, Ka-Ka, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Compilation.h
cfe/trunk/lib/Driver/Compilation.cpp
cfe/trunk/lib/Driver/Driver.cpp

Modified: cfe/trunk/include/clang/Driver/Compilation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=333637&r1=333636&r2=333637&view=diff
==
--- cfe/trunk/include/clang/Driver/Compilation.h (original)
+++ cfe/trunk/include/clang/Driver/Compilation.h Thu May 31 02:05:22 2018
@@ -122,6 +122,9 @@ class Compilation {
   /// Whether an error during the parsing of the input args.
   bool ContainsError;
 
+  /// Whether to keep temporary files regardless of -save-temps.
+  bool ForceKeepTempFiles = false;
+
 public:
   Compilation(const Driver &D, const ToolChain &DefaultToolChain,
   llvm::opt::InputArgList *Args,

Modified: cfe/trunk/lib/Driver/Compilation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=333637&r1=333636&r2=333637&view=diff
==
--- cfe/trunk/lib/Driver/Compilation.cpp (original)
+++ cfe/trunk/lib/Driver/Compilation.cpp Thu May 31 02:05:22 2018
@@ -45,6 +45,11 @@ Compilation::Compilation(const Driver &D
 }
 
 Compilation::~Compilation() {
+  // Remove temporary files. This must be done before arguments are freed, as
+  // the file names might be derived from the input arguments.
+  if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
   delete TranslatedArgs;
   delete Args;
 
@@ -245,6 +250,10 @@ void Compilation::initCompilationForDiag
   AllActions.clear();
   Jobs.clear();
 
+  // Remove temporary files.
+  if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
   // Clear temporary/results file lists.
   TempFiles.clear();
   ResultFiles.clear();
@@ -262,6 +271,9 @@ void Compilation::initCompilationForDiag
 
   // Redirect stdout/stderr to /dev/null.
   Redirects = {None, {""}, {""}};
+
+  // Temporary files added by diagnostics should be kept.
+  ForceKeepTempFiles = true;
 }
 
 StringRef Compilation::getSysRoot() const {

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=333637&r1=333636&r2=333637&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu May 31 02:05:22 2018
@@ -1253,9 +1253,6 @@ void Driver::generateCompilationDiagnost
 
   // If any of the preprocessing commands failed, clean up and exit.
   if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
-  C.CleanupFileList(C.getTempFiles(), true);
-
 Diag(clang::diag::note_drv_command_failed_diag_msg)
 << "Error generating preprocessed source(s).";
 return;
@@ -1372,9 +1369,6 @@ int Driver::ExecuteCompilation(
 
   C.ExecuteJobs(C.getJobs(), FailingCommands);
 
-  // Remove temp files.
-  C.CleanupFileList(C.getTempFiles());
-
   // If the command succeeded, we are done.
   if (FailingCommands.empty())
 return 0;


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

[clang-tools-extra] r333637 - [Driver] Clean up tmp files when deleting Compilation objects

2018-05-31 Thread David Stenberg via cfe-commits 
Author: dstenb
Date: Thu May 31 02:05:22 2018
New Revision: 333637

URL: http://llvm.org/viewvc/llvm-project?rev=333637&view=rev
Log:
[Driver] Clean up tmp files when deleting Compilation objects

Summary:
In rL327851 the createUniqueFile() and createTemporaryFile()
variants that do not return the file descriptors were changed to
create empty files, rather than only check if the paths are free.
This change was done in order to make the functions race-free.

That change led to clang-tidy (and possibly other tools) leaving
behind temporary assembly files, of the form placeholder-*, when
using a target that does not support the internal assembler.

The temporary files are created when building the Compilation
object in stripPositionalArgs(), as a part of creating the
compilation database for the arguments after the double-dash. The
files are created by Driver::GetNamedOutputPath().

Fix this issue by cleaning out temporary files at the deletion of
Compilation objects.

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

Reviewers: klimek, sepavloff, arphaman, aaron.ballman, john.brawn, mehdi_amini, 
sammccall, bkramer, alexfh, JDevlieghere

Reviewed By: aaron.ballman, JDevlieghere

Subscribers: erichkeane, lebedev.ri, Ka-Ka, cfe-commits

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

Added:
clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp

Added: clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp?rev=333637&view=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/pr37091.cpp Thu May 31 02:05:22 2018
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// This is a reproducer for PR37091.
+//
+// Verify that no temporary files are left behind by the clang-tidy invocation.
+
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t clang-tidy %s -- --target=mips64
+// RUN: rmdir %t


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

r355592 - [analyzer] Handle comparison between non-default AS symbol and constant

2019-03-07 Thread David Stenberg via cfe-commits 
Author: dstenb
Date: Thu Mar  7 05:01:17 2019
New Revision: 355592

URL: http://llvm.org/viewvc/llvm-project?rev=355592&view=rev
Log:
[analyzer] Handle comparison between non-default AS symbol and constant

Summary:
When comparing a symbolic region and a constant, the constant would be
widened or truncated to the width of a void pointer, meaning that the
constant could be incorrectly truncated when handling symbols for
non-default address spaces. In the attached test case this resulted in a
false positive since the constant was truncated to zero. To fix this,
widen/truncate the constant to the width of the symbol expression's
type.

This commit does not consider non-symbolic regions as I'm not sure how
to generalize getting the type there.

This fixes PR40814.

Reviewers: NoQ, zaks.anna, george.karpenkov

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, jdoerfert, Charusso, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=355592&r1=355591&r2=355592&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Thu Mar  7 05:01:17 
2019
@@ -571,7 +571,15 @@ SVal SimpleSValBuilder::evalBinOpNN(Prog
   // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
   // then pack it back into a LocAsInteger.
   llvm::APSInt i = rhs.castAs().getValue();
-  BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
+  // If the region has a symbolic base, pay attention to the type; it
+  // might be coming from a non-default address space. For non-symbolic
+  // regions it doesn't matter that much because such comparisons would
+  // most likely evaluate to concrete false anyway. FIXME: We might
+  // still need to handle the non-comparison case.
+  if (SymbolRef lSym = lhs.getAsLocSymbol(true))
+BasicVals.getAPSIntType(lSym->getType()).apply(i);
+  else
+BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
   return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
 }
 default:

Added: cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl?rev=355592&view=auto
==
--- cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl (added)
+++ cfe/trunk/test/Analysis/ptr-cmp-const-trunc.cl Thu Mar  7 05:01:17 2019
@@ -0,0 +1,11 @@
+//RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown -analyze 
-analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+#include 
+
+void bar(__global int *p) __attribute__((nonnull(1)));
+
+void foo(__global int *p) {
+  if ((uint64_t)p <= 1UL << 32)
+bar(p); // no-warning
+}


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

Re: [clang] d9b9621 - Reland D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-23 Thread David Stenberg via cfe-commits 
Hi!

On Mon, 2020-03-23 at 08:56 +0100, Djordje wrote:
> (+ CCing some debug info folks as well)
> 
> Hi David,
> 
> OK, I agree, my apologize.
> 
> The patch enables the whole feature by default and removes the experimental
> option that has been used during the implementation.
> We tested the code (and the assertions regarding the feature itself) by using
> the experimental option on some large projects, but since the feature became
> that huge (and new patches regarding the feature are coming every day -- which
> is nice), it is impossible to test everything.

When the issue related to D75036 was reported I probably should have reverted
that commit directly when starting to troubleshoot the issue, so that we didn't
have to end up reverting D73534.

Sorry about that!

> The Debug Entry Values implementation grows these days, and new 
> functionalities
> comes in, but it has all been tested with the experimental option, so
> committing this particular patch imposes all the cases developers oversighted
> during the implementation.
> 
> My impression is that, before adding new functionalities to the feature, we
> should make sure the D73534 is stable and gets stuck. We should have done it
> before, but I think that is the lesson learned here.

That sounds reasonable I think. For how long should we do that? A week? Two
weeks?

> In general, experimental options are very good up to the some point, but if 
> the
> feature being tested grows a lot, a "breakpoint" should be set up and take 
> care
> the experimental option is removed and the code is well tested on the real
> cases out there, and then moving to the next stage of the implementation.
> 
> Besides this, CISCO is using the feature ON by default for some time, and we
> have good experience there. This really improves the debugging user experience
> while debugging the "optimized" code.
> 
> Best regards,
> Djordje

Best regards,
David S

> On 22.3.20. 16:30, David Blaikie wrote:
> > Also, this patch was reverted and recommitted several times (more than I
> > think would be ideal) - please include more details in the commit message
> > about what went wrong, what was fixed, what testing was missed in the first
> > place and done in subsequent precommit validation (& is there anything more
> > broadly we could learn from this patches story to avoid this kind of
> > revert/recommit repetition in the future?)
> > 
> > On Sat, Mar 21, 2020 at 8:13 PM David Blaikie  > dblai...@gmail.com>> wrote:
> > 
> > Please include the "Differential Revision" line so that Phab picks up
> > commits like this and ties them into the review (& also makes it 
> > conveniently
> > clickable to jump from the commit mail to the review)
> > 
> > On Thu, Mar 19, 2020 at 5:58 AM Djordje Todorovic via cfe-commits <
> > cfe-commits@lists.llvm.org > wrote:
> > 
> > 
> > Author: Djordje Todorovic
> > Date: 2020-03-19T13:57:30+01:00
> > New Revision: d9b962100942c71a4c26debaa716f7ab0c4ea8a1
> > 
> > URL: 
> > https://protect2.fireeye.com/v1/url?k=77e8c227-2b3ccf55-77e882bc-86859b2931b3-a6817f347a752f02&q=1&e=88d8925e-f175-4c81-9485-741ddc24573f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Fd9b962100942c71a4c26debaa716f7ab0c4ea8a1
> > DIFF: 
> > https://protect2.fireeye.com/v1/url?k=3914be57-65c0b325-3914fecc-86859b2931b3-b52b8ad09d192520&q=1&e=88d8925e-f175-4c81-9485-741ddc24573f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Fd9b962100942c71a4c26debaa716f7ab0c4ea8a1.diff
> > 
> > LOG: Reland D73534: [DebugInfo] Enable the debug entry values 
> > feature
> > by default
> > 
> > The issue that was causing the build failures was fixed with the
> > D76164.
> > 
> > Added:
> > llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll
> > 
> > Modified:
> > clang/include/clang/Basic/CodeGenOptions.def
> > clang/include/clang/Driver/CC1Options.td
> > clang/lib/CodeGen/BackendUtil.cpp
> > clang/lib/CodeGen/CGDebugInfo.cpp
> > clang/lib/Frontend/CompilerInvocation.cpp
> > clang/test/CodeGen/debug-info-extern-call.c
> > clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
> > lldb/packages/Python/lldbsuite/test/decorators.py
> >
> > lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Make
> > file
> > llvm/include/llvm/Target/TargetMachine.h
> > llvm/include/llvm/Target/TargetOptions.h
> > llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> > llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
> > llvm/lib/CodeGen/CommandFlags.cpp
> > llvm/lib/CodeGen/LiveDebugValues.cpp
> > llvm/lib/CodeGen/TargetOptionsImpl.cpp
> > llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
> > llvm/lib/Target/ARM/ARMTargetMachine.cpp
> > llvm/lib/

[clang] [clang][CompundLiteralExpr] Don't defer evaluation for CLEs (PR #137163)

2025-07-08 Thread David Stenberg via cfe-commits 

dstenb wrote:

Hi, @kadircet!

This commit broke one of our downstream tests where we mount the repository as 
read-only, as the `static-compound-literals-crash.cpp` test attempts to write 
the output to the test source directory:

```
(frontend): unable to open output file 
'/path/to/repo/clang/test/AST/static-compound-literals-crash.ll': 'Read-only 
file system'
```

It looks like the `RUN` line is missing `-o` or a redirection:

```
RUN: not --crash %clang_cc1 -verify -std=c++20 -emit-llvm %s
```

https://github.com/llvm/llvm-project/pull/137163
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits