[PATCH] D50870: Close FileEntries of cached files in ModuleManager::addModule().

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: bruno, rsmith, teemperor.
Herald added a subscriber: llvm-commits.

Close FileEntries of cached files in ModuleManager::addModule().

While investigating why LLDB (which can build hundreds of clang
modules during one debug session) was getting "too many open files"
errors, I found that most of them are .pcm files that are kept open by
ModuleManager. Pretty much all of the open file dscriptors are
FileEntries that are refering to `.pcm` files for which a buffer
already exists in a CompilerInstance's PCMCache.

Before PCMCache was added it was necessary to hold on to open file
descriptors to ensure that all ModuleManagers using the same
FileManager read the a consistent version of a given `.pcm` file on
disk, even when a concurrent clang process overwrites the file halfway
through. The PCMCache makes this practice unnecessary, since it caches
the entire contents of a `.pcm` file, while the FileManager caches all
the stat() information.

This patch adds a call to FileEntry::closeFile() to the path where a
Buffer has already been created. This is necessary because even for a
freshly written `.pcm` file the file is stat()ed once immediately
after writing to generate a FileEntry in the FileManager. Because a
freshly-generated file's contents is stored in the PCMCache, it is
fine to close the file immediately thereafter.  The second change this
patch makes is to set the `ShouldClose` flag to true when reading a
`.pcm` file into the PCMCache for the first time.

[For reference, in 1 Clang instance there is

- 1 FileManager and
- n ModuleManagers with
- n PCMCaches.]

rdar://problem/40906753


Repository:
  rL LLVM

https://reviews.llvm.org/D50870

Files:
  lib/Serialization/ModuleManager.cpp


Index: lib/Serialization/ModuleManager.cpp
===
--- lib/Serialization/ModuleManager.cpp
+++ lib/Serialization/ModuleManager.cpp
@@ -161,21 +161,24 @@
   if (std::unique_ptr Buffer = lookupBuffer(FileName)) {
 // The buffer was already provided for us.
 NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer));
+// Since the cached buffer is reused, it is safe to close the file
+// descriptor that was opened while stat()ing the PCM in
+// lookupModuleFile() above, it won't be needed any longer.
+Entry->closeFile();
   } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) {
 NewModule->Buffer = Buffer;
+// As above, the file descriptor is no longer needed.
+Entry->closeFile();
   } else {
 // Open the AST file.
 llvm::ErrorOr> 
Buf((std::error_code()));
 if (FileName == "-") {
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
-  // Leave the FileEntry open so if it gets read again by another
-  // ModuleManager it must be the same underlying file.
-  // FIXME: Because FileManager::getFile() doesn't guarantee that it will
-  // give us an open file, this may not be 100% reliable.
+  // Get a buffer of the file and close the file descriptor when done.
   Buf = FileMgr.getBufferForFile(NewModule->File,
  /*IsVolatile=*/false,
- /*ShouldClose=*/false);
+ /*ShouldClose=*/true);
 }
 
 if (!Buf) {


Index: lib/Serialization/ModuleManager.cpp
===
--- lib/Serialization/ModuleManager.cpp
+++ lib/Serialization/ModuleManager.cpp
@@ -161,21 +161,24 @@
   if (std::unique_ptr Buffer = lookupBuffer(FileName)) {
 // The buffer was already provided for us.
 NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer));
+// Since the cached buffer is reused, it is safe to close the file
+// descriptor that was opened while stat()ing the PCM in
+// lookupModuleFile() above, it won't be needed any longer.
+Entry->closeFile();
   } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) {
 NewModule->Buffer = Buffer;
+// As above, the file descriptor is no longer needed.
+Entry->closeFile();
   } else {
 // Open the AST file.
 llvm::ErrorOr> Buf((std::error_code()));
 if (FileName == "-") {
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
-  // Leave the FileEntry open so if it gets read again by another
-  // ModuleManager it must be the same underlying file.
-  // FIXME: Because FileManager::getFile() doesn't guarantee that it will
-  // give us an open file, this may not be 100% reliable.
+  // Get a buffer of the file and close the file descriptor when done.
   Buf = FileMgr.getBufferForFile(NewModule->File,
  /*IsVolatile=*/false,
- /*ShouldClose=*/false);
+ /*ShouldClose=*/true);
 }
 
 if (!Buf) {
___

[PATCH] D50870: Close FileEntries of cached files in ModuleManager::addModule().

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

@bruno: When we last discussed this my plan was to avoid the stat() in 
lookupModuleFile() for files that were just added to the PCMCache by WriteAST() 
entirely, but ModuleManager::Modules is a DenseMap and 
lookupModuleFile() is the easiest way to create a new FileEntry. It would be 
nice to find a way to avoid the stat() for a file that we just wrote, but it 
wasn't immediately obvious to me how to do that.


Repository:
  rL LLVM

https://reviews.llvm.org/D50870



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

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Once we encounter a # directive we are (most likely) looking at some form of 
preprocessed source and that means that the checksum will inevitably be 
different than what we would have gotten were we reading the file directly, 
because of the preprocessing. At this point the value of the hash approaches 
zero. I think dropping all checksums is reasonable in that situation.


Repository:
  rC Clang

https://reviews.llvm.org/D47260



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

vsk wrote:
> I think we need to be a bit more careful here. The current debug location 
> stored in the builder may not be an artificial 0-location. This may cause 
> non-linear single-stepping behavior. Consider this example:
> 
> ```
> void foo() {
>   bar();
>   if (...) {
> int var = ...; //< Clang emits an alloca for "var".
>   }
> ...
> ```
> 
> The current debug location at the line "int var = ..." would be at line 4. 
> But the alloca is emitted in the entry block of the function. In the 
> debugger, this may result in strange single-stepping behavior when stepping 
> into foo(). You could step to line 4, then line 2, then line 3, then line 4 
> again.
> 
> I think we can avoid that by setting an artificial location on allocas.
> I think we can avoid that by setting an artificial location on allocas.
An alloca doesn't really generate any code (or rather.. the code it generates 
is in the function prologue). In what situation would the debug location on an 
alloca influence stepping? Are you thinking about the alloca() function?


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > I think we need to be a bit more careful here. The current debug location 
> > > stored in the builder may not be an artificial 0-location. This may cause 
> > > non-linear single-stepping behavior. Consider this example:
> > > 
> > > ```
> > > void foo() {
> > >   bar();
> > >   if (...) {
> > > int var = ...; //< Clang emits an alloca for "var".
> > >   }
> > > ...
> > > ```
> > > 
> > > The current debug location at the line "int var = ..." would be at line 
> > > 4. But the alloca is emitted in the entry block of the function. In the 
> > > debugger, this may result in strange single-stepping behavior when 
> > > stepping into foo(). You could step to line 4, then line 2, then line 3, 
> > > then line 4 again.
> > > 
> > > I think we can avoid that by setting an artificial location on allocas.
> > > I think we can avoid that by setting an artificial location on allocas.
> > An alloca doesn't really generate any code (or rather.. the code it 
> > generates is in the function prologue). In what situation would the debug 
> > location on an alloca influence stepping? Are you thinking about the 
> > alloca() function?
> An alloca instruction can lower to a subtraction (off the stack pointer) 
> though: https://godbolt.org/g/U4nGzJ.
> 
> `dwarfdump` shows that this subtraction instruction is actually assigned a 
> location -- it currently happens to be the first location in the body of the 
> function. I thought that assigning an artificial location to the alloca would 
> be a first step towards fixing this.
> 
> Also, using an artificial location could mitigate possible bad interactions 
> between code motion passes and IRBuilder. If, say, we were to set the 
> insertion point right after an alloca, we might infer some arbitrary debug 
> location. So long as this inference happens, it seems safer to infer an 
> artificial location.
> 
> 
This may have unintended side-effects: By assigning a debug location to an 
alloca you are moving the end of the function prolog to before the alloca 
instructions, since LLVM computes the end of the function prologue as the first 
instruction with a non-empty debug location. Moving the end of the function 
prologue to before that stack pointer is adjusted is wrong, since that's the 
whole point of the prologue_end marker.

To me it looks more like a bug in a much later stage. With the exception of 
shrink-wrapped code, the prologue_end should always be after the stack pointer 
adjustment, I think.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > aprantl wrote:
> > > > vsk wrote:
> > > > > I think we need to be a bit more careful here. The current debug 
> > > > > location stored in the builder may not be an artificial 0-location. 
> > > > > This may cause non-linear single-stepping behavior. Consider this 
> > > > > example:
> > > > > 
> > > > > ```
> > > > > void foo() {
> > > > >   bar();
> > > > >   if (...) {
> > > > > int var = ...; //< Clang emits an alloca for "var".
> > > > >   }
> > > > > ...
> > > > > ```
> > > > > 
> > > > > The current debug location at the line "int var = ..." would be at 
> > > > > line 4. But the alloca is emitted in the entry block of the function. 
> > > > > In the debugger, this may result in strange single-stepping behavior 
> > > > > when stepping into foo(). You could step to line 4, then line 2, then 
> > > > > line 3, then line 4 again.
> > > > > 
> > > > > I think we can avoid that by setting an artificial location on 
> > > > > allocas.
> > > > > I think we can avoid that by setting an artificial location on 
> > > > > allocas.
> > > > An alloca doesn't really generate any code (or rather.. the code it 
> > > > generates is in the function prologue). In what situation would the 
> > > > debug location on an alloca influence stepping? Are you thinking about 
> > > > the alloca() function?
> > > An alloca instruction can lower to a subtraction (off the stack pointer) 
> > > though: https://godbolt.org/g/U4nGzJ.
> > > 
> > > `dwarfdump` shows that this subtraction instruction is actually assigned 
> > > a location -- it currently happens to be the first location in the body 
> > > of the function. I thought that assigning an artificial location to the 
> > > alloca would be a first step towards fixing this.
> > > 
> > > Also, using an artificial location could mitigate possible bad 
> > > interactions between code motion passes and IRBuilder. If, say, we were 
> > > to set the insertion point right after an alloca, we might infer some 
> > > arbitrary debug location. So long as this inference happens, it seems 
> > > safer to infer an artificial location.
> > > 
> > > 
> > This may have unintended side-effects: By assigning a debug location to an 
> > alloca you are moving the end of the function prolog to before the alloca 
> > instructions, since LLVM computes the end of the function prologue as the 
> > first instruction with a non-empty debug location. Moving the end of the 
> > function prologue to before that stack pointer is adjusted is wrong, since 
> > that's the whole point of the prologue_end marker.
> > 
> > To me it looks more like a bug in a much later stage. With the exception of 
> > shrink-wrapped code, the prologue_end should always be after the stack 
> > pointer adjustment, I think.
> Thanks for explaining, I didn't realize that's how the end of the function 
> prologue is computed! Should we leave out the any debug location changes for 
> allocas in this patch, then?
I think that would be better. You might want to double-check 
PrologueEpilogueInserter and how the FrameSetup attribute is attached to 
MachineInstrs in case my knowledge is out-of-date.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D47097#223, @gramanas wrote:

> In https://reviews.llvm.org/D47097#149, @probinson wrote:
>
> > Can we step back a second and better explain what the problem is? With 
> > current Clang the debug info for this function looks okay to me.
> >  The store that is "missing" a debug location is homing the formal 
> > parameter to its local stack location; this is part of prolog setup, not 
> > "real" code.
>
>
> Isn't this the reason the artificial debug loc exists? The store in the 
> prolog might not be real code but it should still have the scope that the 
> rest of the function has.


Instructions that are part of the function prologue are supposed to have no 
debug location, not an artificial one. The funciton prologue ends at the first 
instruction with a nonempty location.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

[Adrian Prantl via Phabricator via cfe-commits]

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

Minor comment inline.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:378
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;

Can you add a comment explaining why we are doing this here?


https://reviews.llvm.org/D47260



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

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Does this have to be exposed through the driver or could this be a cc1 option 
only?




Comment at: include/clang/Basic/LangOptions.def:144
 BENIGN_LANGOPT(EmitAllDecls  , 1, 0, "emitting all declarations")
+BENIGN_LANGOPT(EmitFwdTemplateChildren, 1, 0, "emit template parameter 
children in forward declarations")
 LANGOPT(MathErrno , 1, 1, "errno in math functions")

Why is this a LangOpt instead of a CodeGenOpt?
Should it reference `debug` somewhere in the name?


https://reviews.llvm.org/D14358



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

[PATCH] D38042: EmitAssemblyHelper: CodeGenOpts.DisableLLVMOpts should not overrule CodeGenOpts.VerifyModule.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl abandoned this revision.
aprantl added a comment.

Abandoning in favor of https://reviews.llvm.org/D38184


https://reviews.llvm.org/D38042



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

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

This needs some kind of testcase. Either a unittest or a test-only executable 
similar to, for example, clang-index-test.


https://reviews.llvm.org/D38473



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

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I can commit this for you if John is happy with it.


https://reviews.llvm.org/D38473



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

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: llvm/include/llvm/Support/Allocator.h:292
+const char *P = reinterpret_cast(Ptr);
+for (size_t Idx=0; Idx < Slabs.size(); Idx++) {
+  const char *S = reinterpret_cast(Slabs[Idx]);

clang-format?



Comment at: llvm/include/llvm/Support/Allocator.h:295
+  if (P >= S && P < S + computeSlabSize(Idx))
+return std::make_pair(Idx, P - S);
+}

`return {Idx, P - S};`


https://reviews.llvm.org/D51393



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

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Just for consideration: The raw pointers in dumps are sometimes useful while in 
a debugger session, because you can cast a pointer and dump the object in the 
debugger.


https://reviews.llvm.org/D51393



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

[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

This is DWARF5+ only, right? (We shouldn't change the preference of Apple 
accelerator tables for DWARF 4 and earlier).


Repository:
  rC Clang

https://reviews.llvm.org/D51576



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

[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D51576#1223562, @labath wrote:

> The interactions here are a bit weird, but the short answer is no, this will 
> not affect apple tables in any way.


Then I have no problem with this patch :-)


Repository:
  rC Clang

https://reviews.llvm.org/D51576



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

[PATCH] D51807: Remove all uses of DIFlagBlockByrefStruct from Clang

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added a reviewer: debug-info.
Herald added a subscriber: JDevlieghere.

This patch removes the last reason why DIFlagBlockByrefStruct from Clang by 
directly implementing the drilling into the member type done in  
DwarfDebug::DbgVariable::getType() into the frontend.


https://reviews.llvm.org/D51807

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenObjC/block-byref-debuginfo.m

Index: test/CodeGenObjC/block-byref-debuginfo.m
===
--- test/CodeGenObjC/block-byref-debuginfo.m
+++ test/CodeGenObjC/block-byref-debuginfo.m
@@ -1,16 +1,35 @@
 // RUN: %clang_cc1 -fblocks -fobjc-arc -fobjc-runtime-has-weak -debug-info-kind=limited -triple x86_64-apple-darwin -emit-llvm %s -o - | FileCheck %s
 
-// rdar://problem/14386148
+// CHECK: !DILocalVariable(name: "foo", {{.*}}type: ![[FOOTY:[0-9]+]])
+// CHECK: ![[FOOTY]] = {{.*}}!DICompositeType({{.*}}, name: "Foo"
+
+// CHECK-NOT: DIFlagBlockByrefStruct
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "__block_literal_1",
+// CHECK-SAME: size: 320, elements: ![[BL_ELTS:[0-9]+]])
+// CHECK: ![[BL_ELTS]] = !{{.*}}![[WFOO:[0-9]+]]}
+
 // Test that the foo is aligned at an 8 byte boundary in the DWARF
 // expression (256) that locates it inside of the byref descriptor:
-// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "foo",
-// CHECK-NOT:line:
-// CHECK-SAME:   offset: 256
+// CHECK: ![[WFOO]] = !DIDerivedType(tag: DW_TAG_member, name: "foo",
+// CHECK-SAME:   baseType: ![[PTR:[0-9]+]]
+// CHECK-SAME:   offset: 256)
+
+// CHECK: ![[PTR]] = !DIDerivedType(tag: DW_TAG_pointer_type,
+// CHECK-SAME:  baseType: ![[WRAPPER:[0-9]+]]
+// CHECK: ![[WRAPPER]] = !DICompositeType(tag: DW_TAG_structure_type, scope:
+// CHECK: elements: ![[WR_ELTS:[0-9]+]])
+// CHECK: ![[WR_ELTS]] = !{{.*}}![[WFOO:[0-9]+]]}
+// CHECK: ![[WFOO]] = !DIDerivedType(tag: DW_TAG_member, name: "foo",
+// CHECK-SAME:   baseType: ![[FOOTY]]
+
+// CHECK: !DILocalVariable(name: "foo", {{.*}}type: ![[FOOTY]])
+
 
 struct Foo {
   unsigned char *data;
 };
 int func() {
   __attribute__((__blocks__(byref))) struct Foo foo;
+  ^{ foo.data = 0; }();
   return 0;
 }
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -491,9 +491,16 @@
  llvm::Optional ArgNo,
  CGBuilderTy &Builder);
 
+  struct BlockByRefType {
+/// The wrapper struct used inside the __block_literal struct.
+llvm::DIType *BlockByRefWrapper;
+/// The type as it appears in the source code.
+llvm::DIType *WrappedType;
+  };
+
   /// Build up structure info for the byref.  See \a BuildByRefType.
-  llvm::DIType *EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
- uint64_t *OffSet);
+  BlockByRefType EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
+  uint64_t *OffSet);
 
   /// Get context info for the DeclContext of \p Decl.
   llvm::DIScope *getDeclContextDescriptor(const Decl *D);
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3565,9 +3565,9 @@
 DBuilder.finalizeSubprogram(Fn->getSubprogram());
 }
 
-llvm::DIType *CGDebugInfo::EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
-uint64_t *XOffset) {
-
+CGDebugInfo::BlockByRefType
+CGDebugInfo::EmitTypeForVarWithBlocksAttr(const VarDecl *VD,
+  uint64_t *XOffset) {
   SmallVector EltTys;
   QualType FType;
   uint64_t FieldSize, FieldOffset;
@@ -3619,23 +3619,21 @@
   }
 
   FType = Type;
-  llvm::DIType *FieldTy = getOrCreateType(FType, Unit);
+  llvm::DIType *WrappedTy = getOrCreateType(FType, Unit);
   FieldSize = CGM.getContext().getTypeSize(FType);
   FieldAlign = CGM.getContext().toBits(Align);
 
   *XOffset = FieldOffset;
-  FieldTy = DBuilder.createMemberType(Unit, VD->getName(), Unit, 0, FieldSize,
-  FieldAlign, FieldOffset,
-  llvm::DINode::FlagZero, FieldTy);
+  llvm::DIType *FieldTy = DBuilder.createMemberType(
+  Unit, VD->getName(), Unit, 0, FieldSize, FieldAlign, FieldOffset,
+  llvm::DINode::FlagZero, WrappedTy);
   EltTys.push_back(FieldTy);
   FieldOffset += FieldSize;
 
   llvm::DINodeArray Elements = DBuilder.getOrCreateArray(EltTys);
-
-  llvm::DINode::DIFlags Flags = llvm::DINode::FlagBlockByrefStruct;
-
-  return DBuilder.createStructType(Unit, "", Unit, 0, FieldOffset, 0, Flags,
-   nullpt

[PATCH] D51807: Remove all uses of DIFlagBlockByrefStruct from Clang

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

( https://reviews.llvm.org/D51763 is not *really* a dependency, but it's 
closely related. )


https://reviews.llvm.org/D51807



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

[PATCH] D51910: [Modules] Add platform feature to requires clause

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: docs/Modules.rst:470
+*platform variant*
+  A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, 
``win32``, ``linux``, etc) is available.
 

Does this work with platforms+environment combinations, such as the ios 
simulator on Darwin?


Repository:
  rC Clang

https://reviews.llvm.org/D51910



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

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

[Adrian Prantl via Phabricator via cfe-commits]

aprantl requested changes to this revision.
aprantl added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369
+  ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field)
+  : 0;
 } else {

It might help to attempt some git blame archeology.
Judging from the comment, it sounds like the debugger is supposed to query the 
runtime for the *byte* offset and then add the bit offset from DWARF? Could 
that make sense?


Repository:
  rC Clang

https://reviews.llvm.org/D51990



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

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369
+  ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field)
+  : 0;
 } else {

aprantl wrote:
> It might help to attempt some git blame archeology.
> Judging from the comment, it sounds like the debugger is supposed to query 
> the runtime for the *byte* offset and then add the bit offset from DWARF? 
> Could that make sense?
If that is the case, we'd need to relax llvm-dwarfdump --verify to accept this 
and make sure LLDB does the right thing instead.


Repository:
  rC Clang

https://reviews.llvm.org/D51990



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

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369
+  ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field)
+  : 0;
 } else {

JDevlieghere wrote:
> JDevlieghere wrote:
> > aprantl wrote:
> > > aprantl wrote:
> > > > It might help to attempt some git blame archeology.
> > > > Judging from the comment, it sounds like the debugger is supposed to 
> > > > query the runtime for the *byte* offset and then add the bit offset 
> > > > from DWARF? Could that make sense?
> > > If that is the case, we'd need to relax llvm-dwarfdump --verify to accept 
> > > this and make sure LLDB does the right thing instead.
> > Ah I see, yeah that sounds reasonable and explains the comment which I 
> > interpreted differently. Thanks! 
> btw it was added in rL167503. 
We should check whether emitting the offsets like this violates the DWARF spec. 
If yes, it may be better to emit the static offsets like you are doing here and 
then still have LLDB ignore everything but the bit-offsets from the dynamic 
byte offset.


Repository:
  rC Clang

https://reviews.llvm.org/D51990



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

[PATCH] D50089: [DWARF v4] Suppressing the __debug_ranges section when there are no ranges

[Adrian Prantl via Phabricator via cfe-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: apple-accel.cpp:11
 
+__attribute__((section("1,__text_foo"))) void foo() {}
 int main (int argc, char const *argv[]) { return argc; }

A comment why this is necessary would be nice.


https://reviews.llvm.org/D50089



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

[PATCH] D50122: Complex Variable defined in InitCapture Crash fix

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: test/SemaCXX/lambda-init-capture-vardefine.cpp:3
+// RUN: %clang_cc1 -std=c++17  -fsyntax-only -verify %s
+// expected-no-diagnostics
+

These kinds of tests that don't check for any output are a bit dangerous, 
because they will also succeed if clang is symlinked to `/bin/true`.


Repository:
  rC Clang

https://reviews.llvm.org/D50122



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

[PATCH] D43044: [DebugInfo] Update Checksum handling in CGDebugInfo

[Adrian Prantl via Phabricator via cfe-commits]

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

SGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D43044



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

[PATCH] D43128: Introduce an API for LLDB to compute the default module cache path

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: bruno, jingham.

LLDB creates Clang modules and had an incomplete copy of the clang Driver code 
that compute the -fmodule-cache-path. This patch makes the clang driver code 
accessible to LLDB.


https://reviews.llvm.org/D43128

Files:
  include/clang/Driver/Driver.h
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2500,6 +2500,13 @@
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -572,6 +572,8 @@
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2500,6 +2500,13 @@
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -572,6 +572,8 @@
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42351: Emit DWARF "constructor" calling convention for every supported Clang CC

[Adrian Prantl via Phabricator via cfe-commits]

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

Looks good with one additional change to the test.

Side note: We should also add all the LLVM extensions to 
http://wiki.dwarfstd.org/index.php?title=Vendor_Extensions.




Comment at: test/CodeGen/debug-info-cc.c:1
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -o - -emit-llvm 
-debug-info-kind=limited %s >%t
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -o - -emit-llvm 
-debug-info-kind=limited %s >>%t

I think I would prefer this to be written as 
```
// RUN: $clang_cc1 ... %s | FileCheck --check-prefix=LINUX
// RUN: $clang_cc1 ... %s | FileCheck --check-prefix=WINDOWS
...


// LINUX: !DISubprogram ...
```


https://reviews.llvm.org/D42351



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

[PATCH] D43189: [DebugInfo] Avoid name conflict of generated VLA expression variable.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/CodeGen/CGDecl.cpp:1002
   getContext().CreateTypeSourceInfo(QT), SC_Auto);
+  ArtificialDecl->setImplicit();
 

Thanks!


https://reviews.llvm.org/D43189



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

[PATCH] D43494: [Modules] Fix creating fake definition data for lambdas.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2969
 
+if (!DD && RD->isBeingDefined())
+  return nullptr;

Perhaps add a comment explaining what's going on in this early exit?


https://reviews.llvm.org/D43494



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

[PATCH] D33705: [CGVTables] Finalize SP before attempting to clone it

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Looks good.. Are you also planning to change DIBuilder to not finalize 
subprograms automatically any more (and not insert them into AllSubprograms)? 
(That will be the more impactful change as it will force all non-clang 
frontends to make a similar change).


https://reviews.llvm.org/D33705



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

[PATCH] D33893: Align definition of DW_OP_plus with DWARF spec [2/3]

[Adrian Prantl via Phabricator via cfe-commits]

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

Thanks, this LGTM!


https://reviews.llvm.org/D33893



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

[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:3010
+return CGF.EmitCheckedInBoundsGEP(ptr, indices, signedIndices,
+  /*IsSubtraction=*/false, loc, name);
   } else {

Might want to define an

`enum { NotSubtraction = false, IsSubtraction = true }` in the header. We do 
this in other places.


https://reviews.llvm.org/D34121



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

[PATCH] D41259: [debuginfo] Remove temporary FIXME.

[Adrian Prantl via Phabricator via cfe-commits]

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

I suppose this is fine, please check on the green dragon builders after 
committing to make sure there's no unexpected fallout.


Repository:
  rC Clang

https://reviews.llvm.org/D41259



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

[PATCH] D41698: [DebugInfo] Enable debug information for C99 VLA types

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

It would be awesome if you could also add an end-to-end test to the 
debuginfo-tests repository so we can verify that this actually works in LLDB 
and GDB.




Comment at: lib/CodeGen/CGDebugInfo.cpp:2358
+if (auto *SizeNode = getVLASizeExpressionForType(EltTy))
+  Subscripts.push_back(DBuilder.getOrCreateSubrange(0, SizeNode));
+else

perhaps write sizeNode/Count to a variable, so you don't have to duplicate the 
expression?



Comment at: lib/CodeGen/CGDebugInfo.cpp:3473
   CGBuilderTy &Builder) {
+  llvm::Optional Optional;
+  EmitDeclare(VD, Storage, ArgNo, Optional, Builder);

Could this function be replaced by a default argument instead?



Comment at: lib/CodeGen/CGDebugInfo.h:86
+  /// represented by instantiated Metadata nodes.
+  llvm::SmallDenseMap SizeExprCache;
+

I'm expecting VLA's to not be very common, should we use a regular DenseMap 
instead?



Comment at: lib/CodeGen/CGDebugInfo.h:317
+  llvm::Metadata *getVLASizeExpressionForType(QualType Ty) {
+if (SizeExprCache.count(Ty))
+  return SizeExprCache[Ty];

This is performing the lookup twice. If you use .find() instead it will be more 
efficient. We also don't use accessor functions like this for any of the other 
caches. If you think that they help, perhaps make this more generic and useful 
for all caches?



Comment at: lib/CodeGen/CGDebugInfo.h:404
+  void EmitDeclareOfAutoVariable(const VarDecl *Decl, llvm::Value *AI,
+ llvm::Optional 
&MetadataDecl,
+ CGBuilderTy &Builder);

do we need the qualifier on Optional?



Comment at: lib/CodeGen/CGDecl.cpp:1125
+// If we have debug info enabled, describe the VLA dimensions properly.
+if (EmitDebugInfo) {
+  QualType Type1D = Ty;

Please move this code into a member function of CGDebugInfo.



Comment at: lib/CodeGen/CGDecl.cpp:1137
+  // Allocate memory for the address of the vla expression
+  // We can use this for debugging purposes
+  auto SizeExprAddr = CreateDefaultAlignTempAlloca(

`.`



Comment at: lib/CodeGen/CodeGenFunction.cpp:1963
+  assert(VlaSize->getType() == SizeTy);
+  return std::pair(VlaSize, Vla->getElementType());
+}

`return {VlaSize, Vla->getElementType()};`


https://reviews.llvm.org/D41698



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

[PATCH] D41743: Debug Info: Support DW_AT_calling_convention on composite types.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: echristo, dblaikie, probinson.
aprantl added a project: debug-info.
Herald added a subscriber: JDevlieghere.

This implements the DWARF 5 feature described at
http://www.dwarfstd.org/ShowIssue.php?issue=141215.1

This allows a consumer to understand whether a composite data type is
trivially copyable and thus should be passed by value instead of by
reference. The canonical example is being able to distinguish the
following two types:

  // S is not trivially copyable because of the explicit destructor.
  struct S {
 ~S() {}
  };
  
  // T is a POD type.
  struct T {
~T() = default;
  };

To avoid bloating the debug info with calling convention attributes,
this patch only adds them were the calling convention is not obvious
from the context.

Implicitly by value is everything that clearly looks like a C struct, i.e.:

- Non-C++ record types.
- Types that define none of destructor, copy/move constructor, copy/move 
assignment operator.

Implicitly by reference is everything clearly looks like a non-pod type:

- Types that define a destructor, a copy constructor, and a copy assignment 
operator.


Repository:
  rL LLVM

https://reviews.llvm.org/D41743

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-composite-cc.cpp

Index: test/CodeGenCXX/debug-info-composite-cc.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-info-composite-cc.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s
+
+// Not trivially copyable because of the explicit destructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefDtor",{{.*size: [0-9]+, elem}}
+struct RefDtor {
+  int i;
+  ~RefDtor() {}
+} refDtor;
+
+// Not trivially copyable because of the explicit copy constructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefCopy",{{.*size: [0-9]+, elem}}
+struct RefCopy {
+  int i;
+  RefCopy() {}
+  RefCopy(RefCopy &Copy) {}
+} refCopy;
+
+// Not trivially copyable because of the explicit move constructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefMove",{{.*}}flags: DIFlagTypePassByReference
+struct RefMove {
+  int i;
+  RefMove(){}
+  RefMove(RefMove &&Move) {}
+} refMove;
+
+// Not trivially copyable because of the explicit copy assignment.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefCopyAss",{{.*size: [0-9]+, elem}}
+struct RefCopyAss {
+  int i;
+  RefCopyAss &operator= (RefCopyAss &Copy) { return *this; }
+} refCopyAss;
+
+
+// POD-like type even though it defines a destructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Podlike", {{.*}}flags: DIFlagTypePassByValue
+struct Podlike {
+  int i;
+  Podlike() = default;
+  Podlike(Podlike &&Move) = default;
+  ~Podlike() = default;
+} podlike;
+
+
+// This is a POD type.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Pod",{{.*size: [0-9]+, elem}}
+struct Pod {
+  int i;
+} pod;
+
+// This is definitely not a POD type.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Complex",{{.*size: [0-9]+, elem}}
+struct Complex {
+  Complex() {}
+  Complex(Complex &Copy) : i(Copy.i) {};
+  int i;
+} complex;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2803,9 +2803,35 @@
 
   SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);
 
+  // Explicitly record the calling convention unless it is obvious
+  // from context.
+  // Implicitly by value is everything that clearly looks like a C struct, i.e.:
+  //   - Non-C++ record types.
+  //   - Types that define none of destructor, copy/move constructor,
+  // copy/move assignment operator.
+  // Implicitly by reference is everything clearly looks like a non-pod type:
+  //  - Types that define a destructor, a copy constructor, and a copy
+  //assignment operator.
+  auto Flags = llvm::DINode::FlagZero;
+  if (auto CXXRD = dyn_cast(RD)) {
+if (CXXRD->isTriviallyCopyable()) {
+  if (CXXRD->hasUserDeclaredDestructor() ||
+  CXXRD->hasUserDeclaredCopyConstructor() ||
+  CXXRD->hasUserDeclaredMoveConstructor() ||
+  CXXRD->hasUserDeclaredCopyAssignment() ||
+  CXXRD->hasUserDeclaredMoveAssignment())
+Flags |= llvm::DINode::FlagTypePassByValue;
+} else {
+  if (!CXXRD->hasUserDeclaredDestructor() &&
+  !CXXRD->hasUserDeclaredCopyConstructor() &&
+  !CXXRD->hasUserDeclaredCopyAssignment())
+Flags |= llvm::DINode::FlagTypePassByReference;
+}
+  }
+
   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(
   getTagForRecord(RD), RDName, RDContext, DefUnit, Line, 0, Size, Align,
-  llvm::DINode::FlagZero, FullName);
+  Flags, FullName);
 
   // Elements of composite types usually have back to the type, creating
   // uniquing cycles.  Distinct nodes are more efficient

[PATCH] D41743: Debug Info: Support DW_AT_calling_convention on composite types.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I related https://reviews.llvm.org/D41039 (Add support for attribute 
"trivial_abi") which will also benefit from this feature. It is not a hard 
dependency though, this patch is also useful for standard C++.


Repository:
  rL LLVM

https://reviews.llvm.org/D41743



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

[PATCH] D41743: Debug Info: Support DW_AT_calling_convention on composite types.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks!  I think you got me convinced that we should probably stick the calling 
convention attribute unconditionally on all C++ types. At least in DWARF 5 it 
should be part of the abbreviation and thus almost free.


Repository:
  rL LLVM

https://reviews.llvm.org/D41743



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

[PATCH] D41743: Debug Info: Support DW_AT_calling_convention on composite types.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 128686.
aprantl added a comment.

Just unconditionally emit the flags for all CXXRecordDecls. Richard convinced 
me that a debugger does not want to be in the business of determining the 
correct calling convention.


https://reviews.llvm.org/D41743

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-composite-cc.cpp


Index: test/CodeGenCXX/debug-info-composite-cc.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-info-composite-cc.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple 
%itanium_abi_triple %s -o - | FileCheck %s
+
+// Not trivially copyable because of the explicit destructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefDtor",{{.*}}flags: 
DIFlagTypePassByReference
+struct RefDtor {
+  int i;
+  ~RefDtor() {}
+} refDtor;
+
+// Not trivially copyable because of the explicit copy constructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefCopy",{{.*}}flags: 
DIFlagTypePassByReference
+struct RefCopy {
+  int i;
+  RefCopy() = default;
+  RefCopy(RefCopy &Copy) {}
+} refCopy;
+
+// Not trivially copyable because of the explicit move constructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefMove",{{.*}}flags: 
DIFlagTypePassByReference
+struct RefMove {
+  int i;
+  RefMove() = default;
+  RefMove(RefMove &&Move) {}
+} refMove;
+
+// POD-like type even though it defines a destructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Podlike", {{.*}}flags: 
DIFlagTypePassByValue
+struct Podlike {
+  int i;
+  Podlike() = default;
+  Podlike(Podlike &&Move) = default;
+  ~Podlike() = default;
+} podlike;
+
+
+// This is a POD type.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Pod",{{.*}}flags: 
DIFlagTypePassByValue
+struct Pod {
+  int i;
+} pod;
+
+// This is definitely not a POD type.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Complex",{{.*}}flags: 
DIFlagTypePassByReference
+struct Complex {
+  Complex() {}
+  Complex(Complex &Copy) : i(Copy.i) {};
+  int i;
+} complex;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2803,9 +2803,18 @@
 
   SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);
 
+  // Explicitly record the calling convention for C++ records.
+  auto Flags = llvm::DINode::FlagZero;
+  if (auto CXXRD = dyn_cast(RD)) {
+if (CGM.getCXXABI().getRecordArgABI(CXXRD) == CGCXXABI::RAA_Indirect)
+  Flags |= llvm::DINode::FlagTypePassByReference;
+else
+  Flags |= llvm::DINode::FlagTypePassByValue;
+  }
+
   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(
   getTagForRecord(RD), RDName, RDContext, DefUnit, Line, 0, Size, Align,
-  llvm::DINode::FlagZero, FullName);
+  Flags, FullName);
 
   // Elements of composite types usually have back to the type, creating
   // uniquing cycles.  Distinct nodes are more efficient.


Index: test/CodeGenCXX/debug-info-composite-cc.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-info-composite-cc.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s
+
+// Not trivially copyable because of the explicit destructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefDtor",{{.*}}flags: DIFlagTypePassByReference
+struct RefDtor {
+  int i;
+  ~RefDtor() {}
+} refDtor;
+
+// Not trivially copyable because of the explicit copy constructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefCopy",{{.*}}flags: DIFlagTypePassByReference
+struct RefCopy {
+  int i;
+  RefCopy() = default;
+  RefCopy(RefCopy &Copy) {}
+} refCopy;
+
+// Not trivially copyable because of the explicit move constructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "RefMove",{{.*}}flags: DIFlagTypePassByReference
+struct RefMove {
+  int i;
+  RefMove() = default;
+  RefMove(RefMove &&Move) {}
+} refMove;
+
+// POD-like type even though it defines a destructor.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Podlike", {{.*}}flags: DIFlagTypePassByValue
+struct Podlike {
+  int i;
+  Podlike() = default;
+  Podlike(Podlike &&Move) = default;
+  ~Podlike() = default;
+} podlike;
+
+
+// This is a POD type.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Pod",{{.*}}flags: DIFlagTypePassByValue
+struct Pod {
+  int i;
+} pod;
+
+// This is definitely not a POD type.
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Complex",{{.*}}flags: DIFlagTypePassByReference
+struct Complex {
+  Complex() {}
+  Complex(Complex &Copy) : i(Copy.i) {};
+  int i;
+} complex;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2803,9 +2803,18 @@
 
   SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, Th

[PATCH] D41039: Add support for attribute "trivial_abi"

[Adrian Prantl via Phabricator via cfe-commits]

aprantl reopened this revision.
aprantl added a comment.

I'm sorry, I made a copy&paste error in a the commit message for 
https://reviews.llvm.org/D41743 and accidentally associated that commit with 
this review. I suppose I should learn to use arcanist.
Reopening, and my apologies for the noise!


Repository:
  rL LLVM

https://reviews.llvm.org/D41039



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

[PATCH] D41039: Add support for attribute "trivial_abi"

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Unfortunately it looks like I do not have permission to re-upload Akira's last 
patch to this review.


Repository:
  rL LLVM

https://reviews.llvm.org/D41039



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

[PATCH] D44605: [Driver] Default to DWARF 5 for Fuchsia

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I should also point out that even in DWARF v5 mode LLVM does not yet emit DWARF 
5 variants of all sections as DWARF v5 support in LLVM is not yet 
feature-complete.


Repository:
  rC Clang

https://reviews.llvm.org/D44605



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

[PATCH] D47720: [DebugInfo] Inline for without DebugLocation

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: test/CodeGen/debug-info-inline-for.c:2
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+int func(int n) {

Please add a comment explaining what is being tested here.


Repository:
  rC Clang

https://reviews.llvm.org/D47720



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

[PATCH] D47720: [DebugInfo] Inline for without DebugLocation

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: test/CodeGen/debug-info-inline-for.c:13
+
+// CHECK: ![[DbgLoc]] = !DILocation(

Shouldn't you also check *which* location is attached to it?


Repository:
  rC Clang

https://reviews.llvm.org/D47720



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1949
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);

Can you make that comment elaborate more about why this is being done? For 
example, you could add an "otherwise mem2reg will ..."

Also please note that all comments in LLVM need to be full sentences with a "." 
at the end :-)


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47996: Added modulemap for lldb-mi

[Adrian Prantl via Phabricator via cfe-commits]

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

That should work. Thanks!


https://reviews.llvm.org/D47996



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

[PATCH] D48241: [DebugInfo] Emit ObjC methods as part of interface.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks! Is there also a companion patch for LLVM that disables the objc 
accelerator table?

Note that this is not a 100% replacement of the apple_objc accelerator table, 
since the apple_objc table also lists all methods defined in categories of that 
interface. Is the idea to also add category methods into the interface's 
DW_TAG_struture_type?




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3358
+// Starting with DWARF5, we create declarations for the interface's
+// methods.
+if (const auto *OMD = dyn_cast_or_null(D)) {

`// Starting with DWARF V5 method declarations are emitted as children of the 
interface type.`



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4247
+
+  SmallVector EltTys;
+  for (auto *E : RealDecl->getElements()) {

`EltTys.append(RealDecl->getElements().begin(), RealDecl->getElements().end())`



Comment at: clang/lib/CodeGen/CGDebugInfo.h:105
+  llvm::DISubprogram *DIMethodDecl;
+  MethodData(const ObjCMethodDecl *MD, llvm::DISubprogram *DIMethodDecl)
+  : MD(MD), DIMethodDecl(DIMethodDecl) {}

This constructor is probably not necessary if you construct the struct as `{ 
MD, Decl }`?



Comment at: clang/lib/CodeGen/CGDebugInfo.h:111
+// methods.
+llvm::DICompositeType *DIInterfaceDecl;
+std::vector Methods;

Isn't the interface already the key in the DenseMap?



Comment at: clang/test/CodeGenObjC/debug-info-synthesis.m:35
 
+// DWARF5: ![[STRUCT:.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"Foo"
 // CHECK: ![[FILE:.*]] = !DIFile(filename: "{{[^"]+}}foo.h"

We should also check that this does not happen in DWARF 4.


Repository:
  rC Clang

https://reviews.llvm.org/D48241



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

[PATCH] D48241: [DebugInfo] Emit ObjC methods as part of interface.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> could you provide some examples (just in comments/description in this code 
> review) of what the DWARF used to look like and what it looks like after this 
> change?

Thus far an Objective-C interface is a DW_TAG_structure_type that only has 
variables and properties as members. Objective-C methods are emitted as 
freestanding DW_TAG_subprogram function definitions. In addition, the 
.apple_objc accelerator table provides a mapping from ClassName -> [list of 
subprogram DIEs for each method in Class or one of its categories].

The idea behind this patch is to get rid of the accelerator table and encode 
the same information in the DIE hierarchy instead. LLDB was never using the 
accelerator table to accelerate lookup but only when it was in the middle of 
parsing a DW_TAG_structure_type of an Objective-C interface, so this shouldn't 
even come add a performance penalty.


https://reviews.llvm.org/D48241



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

[PATCH] D48241: [DebugInfo] Emit ObjC methods as part of interface.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4250
+
+  SmallVector EltTys;
+  EltTys.append(InterfaceDecl->getElements().begin(),

```
auto &Elements = InterfaceDecl->getElements();
EltTys.append(Elements.begin(), Elements.end());
for (auto &M : p.second)
   EltTys.push_back(M.DIMethodDecl);
```



Comment at: clang/lib/CodeGen/CGDebugInfo.h:106
+
+  /// Cache of forward declarations for method
+  llvm::DenseMap>

looks like that comment is cut off at the end?



Comment at: clang/test/CodeGenObjC/debug-info-category.m:41
+// DWARF4-NOT: = !DISubprogram(name: "-[Foo integer:]"{{.*}}isDefinition: false
+// DWARF4-NOT: = !DISubprogram(name: "-[Foo(Bar) add:]"{{.*}}isDefinition: 
false
+

Shouldn't you also check the scope: of the DISubprograms?


https://reviews.llvm.org/D48241



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

[PATCH] D48367: [modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:285
 // directory.
-loadSubdirectoryModuleMaps(SearchDirs[Idx]);
+if (ModMap.getLangOpts().ObjC1 || ModMap.getLangOpts().ObjC2)
+  loadSubdirectoryModuleMaps(SearchDirs[Idx]);

Are these flags also enabled in Objective-C++ mode?


https://reviews.llvm.org/D48367



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

[PATCH] D53329: Generate DIFile with main program if source is not available

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Couldn't you just pass `-main-file-name` to cc1 instead?


Repository:
  rC Clang

https://reviews.llvm.org/D53329



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I have a vague recollection that this column info hack was added to 
disambiguate two types defined on the same line (which is something that 
happened more often than one would think because of macro expansion). Did  you 
do the git archeology to ensure that the original reason for doing it this way 
has been resolved? FWIW, I'm fine with doing this change for the Darwin 
platforms because column info is turned on by default, so this shouldn't affect 
us.


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Oh wait, this patch is just for dumping the ASTs? Can you elaborate why this 
makes it into a binary then?


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

RTTI?


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added a reviewer: vsk.

Function calls without a !dbg location inside a function that has a 
DISubprogram make it impossible to construct inline information and are 
rejected by the verifier. This patch ensures that sanitizer check function 
calls have a !dbg location, by carrying forward the location of the preceding 
instruction or by inserting an artificial location if necessary.

This fixes a crash when compiling the attached testcase with -Os.

rdar://problem/45311226


https://reviews.llvm.org/D53459

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/ubsan-check-debuglocs.cpp


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1 {{.*}}, !dbg
+
+class SourceLocation {
+public:
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool 
IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1 {{.*}}, !dbg
+
+class SourceLocation {
+public:
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:2871
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);

vsk wrote:
> Why shouldn't this always be line 0? A call to a check handler is always 
> auto-generated.
I was thinking that it might be nice to have the source location of the 
expression with the UB in the backtrace, too — not just in the error message. 
(Which is the current behavior by accident, since no location means carry over 
the previous instruction's location). But I'm fine with just setting line 0, 
too.



Comment at: test/CodeGenCXX/ubsan-check-debuglocs.cpp:2
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null,object-size,return -fsanitize-recover=null \
+// RUN:   %s -o - | FileCheck %s

vsk wrote:
> Are all three sanitizers needed here to reproduce the bug? Seems like a 
> simpler test would be:
> 
> ```
> // RUN: ... -fsanitize=null ...
> 
> int deref(int *p) { return *p; }
> // CHECK-LABEL: @deref
> // CHECK: __ubsan_handle_type_mismatch_v1{{.*}} !dbg 
> [[ubsan_handler_loc:![0-9]+]]
> // CHECK: [[ubsan_handler_loc]] = !DILocation(line: 0
> ```
Probably not for the testcase; only to trigger the subsequent crash.


https://reviews.llvm.org/D53459



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

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 170433.
aprantl added a comment.

Simplify testcase


https://reviews.llvm.org/D53459

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/ubsan-check-debuglocs.cpp


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1
+// CHECK-SAME: !dbg
+
+class SourceLocation {
+public:
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool 
IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1
+// CHECK-SAME: !dbg
+
+class SourceLocation {
+public:
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53459: Ensure sanitizer check function calls have a !dbg location

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 170434.
aprantl added a comment.

further simplify testcase


https://reviews.llvm.org/D53459

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/ubsan-check-debuglocs.cpp


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1
+// CHECK-SAME: !dbg
+
+struct SourceLocation {
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool 
IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;


Index: test/CodeGenCXX/ubsan-check-debuglocs.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-check-debuglocs.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited \
+// RUN:   -fsanitize=null %s -o - | FileCheck %s
+
+// Check that santizer check calls have a !dbg location.
+// CHECK: define {{.*}}acquire{{.*}} !dbg
+// CHECK-NOT: define
+// CHECK: call void {{.*}}@__ubsan_handle_type_mismatch_v1
+// CHECK-SAME: !dbg
+
+struct SourceLocation {
+  SourceLocation acquire() {};
+};
+extern "C" void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc);
+static void handleTypeMismatchImpl(SourceLocation *Loc) { Loc->acquire(); }
+void __ubsan_handle_type_mismatch_v1(SourceLocation *Loc) {
+  handleTypeMismatchImpl(Loc);
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2867,6 +2867,9 @@
  CheckRecoverableKind RecoverKind, bool IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
+  auto *DI = CGF.getDebugInfo();
+  SourceLocation Loc = DI ? DI->getLocation() : SourceLocation();
+  auto DL = ApplyDebugLocation::CreateDefaultArtificial(CGF, Loc);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
   bool MinimalRuntime = CGF.CGM.getCodeGenOpts().SanitizeMinimalRuntime;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D45045#1270442, @HsiangKai wrote:

> In https://reviews.llvm.org/D45045#1247427, @vitalybuka wrote:
>
> > Reverted in r343183
> >  https://bugs.llvm.org/show_bug.cgi?id=39094
> >  
> > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/29356/steps/build%20release%20tsan%20with%20clang/logs/stdio
>
>
> I have fixed the bug in https://reviews.llvm.org/D52927 and it has landed in 
> master branch. Could I recommit this patch?


As long as you keep a close eye on the bots that should be fine, yes.


Repository:
  rL LLVM

https://reviews.llvm.org/D45045



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

It would be nice if instead of having a set of windows-only tests, we could 
wrap cdb similar to llgdb.py wraps LLDB, so these tests run on all platforms. 
If the Windows debugger is just too different for this to make sense, let me 
know.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> I think the only way to realistically make this work for all platforms would 
> be to separate the source file from the input/output. The source file would 
> be the test case, and if you wanted to support a different debugger you would 
> need to supply a different input/output file..

I don't necessarily agree with that statement. The debuginfo-tests use only a 
very small subset of debugger functionality that includes

- setting breakpoints
- printing the value of integer variables
- continuing to the next breakpoint

I'm genuinely curious what is so different about the Windows debugger that it 
couldn't be wrapped in a translation layer like `llgdb.py` that abstracts these 
three operations. This would cover a large set of the existing tests. I'm fine 
with having a few extra tests that test something that only works on a specific 
platform here and there, but I really don't want us to grow separate 
platform-specific testsuites. Inevitably, someone working on platform A will 
fix a general bug in LLVM and then only add a test for platform A and that's 
bad for everyone. What I'm trying to avoid is a situation like the debug info 
tests in LLVM that are almost entirely x86-specific, even if they are testing 
stuff that happens at the IR level.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54187#1290293, @zturner wrote:

> Especially since as far as I can tell, nobody has even run debuginfo-tests 
> since late August, because it was actually broken by r341135 on August 30 
> (fixed in r346060 yesterday)


Can you please refrain from making such general statements? They distract from 
the discussion.

http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/ runs the 
debuginfo-tests continuously. There is a configuration issue that prevents it 
from running the ASAN subset of the tests at the moment.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54187#1290298, @probinson wrote:

> In https://reviews.llvm.org/D54187#1290294, @zturner wrote:
>
> > In https://reviews.llvm.org/D54187#1290282, @probinson wrote:
> >
> > > +gbedwell
> > >
> > > Just to throw the idea out there, why not abandon debuginfo-tests 
> > > entirely and try using Dexter for this. Dexter's design center is 
> > > debug-info quality, not correctness, but it already knows how to drive 
> > > several different debuggers on multiple platforms.
> > >  Dexter would have to become an LLVM project tool, and I'm not sure how 
> > > Sony management feels about that, but I think this would be an awesome 
> > > use-case.
> >
> >
> > Depends where we draw the distinction between quality and correctness.  We 
> > specifically want a way to test that when we fix a correctness bug, it's 
> > actually fixed against Microsoft debuggers.
>
>
> Dexter knows how to drive Visual Studio tools already, as well as gdb and 
> (maybe) lldb.  I have never looked inside it but I'd expect Greg to have made 
> it straightforward to add new tools.


If it fits our use-cases, that sounds like a great idea. Are there some 
examples of dexter input/outputs we could take a look at to see how good of a 
fit it would be?


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54187#1290317, @zturner wrote:

> In https://reviews.llvm.org/D54187#1290297, @aprantl wrote:
>
> > In https://reviews.llvm.org/D54187#1290293, @zturner wrote:
> >
> > > Especially since as far as I can tell, nobody has even run 
> > > debuginfo-tests since late August, because it was actually broken by 
> > > r341135 on August 30 (fixed in r346060 yesterday)
> >
> >
> > Can you please refrain from making such general statements? They distract 
> > from the discussion.
> >
> > http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/ runs 
> > the debuginfo-tests continuously. There is a configuration issue that 
> > prevents it from running the ASAN subset of the tests at the moment.
>
>
> The issue introduced by r341135 doesn't seem related to ASAN unless I'm 
> misunderstanding the issue.  Were debuginfo-tests passing on that bot even 
> before r346060 landed?


r341135 is a fix to `lit.site.cfg.py.in` my best guess is that because 
clang-stage1-cmake-RA-expensive runs the debuginfo-tests inside the 
llvm/tools/clang/test/debuginfo-tests subdirectory that doesn't get used in 
that configuration?


https://reviews.llvm.org/D54187



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

[PATCH] D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags

[Adrian Prantl via Phabricator via cfe-commits]

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

Everybody with out-of-tree tests will be excited ;-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54756



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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D44100#1302478 , @a_sidorin wrote:

> I don't mean only review. As I guess, you guys have MacOS machines so you can 
> check if the problem is still present in the updated version.


FYI, typically problems with ASTImporter in LLDB are not macOS-specific at all 
and also reproduce when building LLDB for Linux. You can just check out LLDB 
into the llvm/tools directory and run `ninja && ninja check-lldb`


Repository:
  rC Clang

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

https://reviews.llvm.org/D44100



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

[PATCH] D55037: [-gmodules] Honor -fdebug-prefix-map in the debug info inside PCMs.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: bruno, rsmith.

This patch passes -fdebug-prefix-map (a feature for renaming source paths in 
the debug info) through to the per-module codegen options and adds the debug 
prefix map to the module hash.

  

rdar://problem/46045865


https://reviews.llvm.org/D55037

Files:
  lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Modules/module-debuginfo-prefix.m


Index: test/Modules/module-debuginfo-prefix.m
===
--- /dev/null
+++ test/Modules/module-debuginfo-prefix.m
@@ -0,0 +1,23 @@
+// REQUIRES: asserts
+
+// Modules:
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \
+// RUN:   -fdebug-prefix-map=%S/Inputs=/OVERRIDE \
+// RUN:   -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \
+// RUN:   -I %S/Inputs -I %t -emit-llvm -o %t.ll \
+// RUN:   -mllvm -debug-only=pchcontainer &>%t-mod.ll
+// RUN: cat %t-mod.ll | FileCheck %s
+
+// PCH:
+// RUN: %clang_cc1 -x objective-c -emit-pch -fmodule-format=obj -I %S/Inputs \
+// RUN:   -fdebug-prefix-map=%S/Inputs=/OVERRIDE \
+// RUN:   -o %t.pch %S/Inputs/DebugObjC.h \
+// RUN:   -mllvm -debug-only=pchcontainer &>%t-pch.ll
+// RUN: cat %t-pch.ll | FileCheck %s
+
+#ifdef MODULES
+@import DebugObjC;
+#endif
+
+// CHECK: !DIFile({{.*}}"/OVERRIDE/DebugObjC.h"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3264,6 +3264,12 @@
 code = ext->hashExtension(code);
   }
 
+  // When compiling with -gmodules, also hash -fdebug-prefix-map as it
+  // affects the debug info in the PCM.
+  if (getCodeGenOpts().DebugTypeExtRefs)
+for (const auto &KeyValue : getCodeGenOpts().DebugPrefixMap)
+  code = hash_combine(code, KeyValue.first, KeyValue.second);
+
   // Extend the signature with the enabled sanitizers, if at least one is
   // enabled. Sanitizers which cannot affect AST generation aren't hashed.
   SanitizerSet SanHash = LangOpts->Sanitize;
Index: lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -156,6 +156,8 @@
 LangOpts.CurrentModule.empty() ? MainFileName : LangOpts.CurrentModule;
 CodeGenOpts.setDebugInfo(codegenoptions::FullDebugInfo);
 CodeGenOpts.setDebuggerTuning(CI.getCodeGenOpts().getDebuggerTuning());
+CodeGenOpts.DebugPrefixMap =
+CI.getInvocation().getCodeGenOpts().DebugPrefixMap;
   }
 
   ~PCHContainerGenerator() override = default;


Index: test/Modules/module-debuginfo-prefix.m
===
--- /dev/null
+++ test/Modules/module-debuginfo-prefix.m
@@ -0,0 +1,23 @@
+// REQUIRES: asserts
+
+// Modules:
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \
+// RUN:   -fdebug-prefix-map=%S/Inputs=/OVERRIDE \
+// RUN:   -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \
+// RUN:   -I %S/Inputs -I %t -emit-llvm -o %t.ll \
+// RUN:   -mllvm -debug-only=pchcontainer &>%t-mod.ll
+// RUN: cat %t-mod.ll | FileCheck %s
+
+// PCH:
+// RUN: %clang_cc1 -x objective-c -emit-pch -fmodule-format=obj -I %S/Inputs \
+// RUN:   -fdebug-prefix-map=%S/Inputs=/OVERRIDE \
+// RUN:   -o %t.pch %S/Inputs/DebugObjC.h \
+// RUN:   -mllvm -debug-only=pchcontainer &>%t-pch.ll
+// RUN: cat %t-pch.ll | FileCheck %s
+
+#ifdef MODULES
+@import DebugObjC;
+#endif
+
+// CHECK: !DIFile({{.*}}"/OVERRIDE/DebugObjC.h"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3264,6 +3264,12 @@
 code = ext->hashExtension(code);
   }
 
+  // When compiling with -gmodules, also hash -fdebug-prefix-map as it
+  // affects the debug info in the PCM.
+  if (getCodeGenOpts().DebugTypeExtRefs)
+for (const auto &KeyValue : getCodeGenOpts().DebugPrefixMap)
+  code = hash_combine(code, KeyValue.first, KeyValue.second);
+
   // Extend the signature with the enabled sanitizers, if at least one is
   // enabled. Sanitizers which cannot affect AST generation aren't hashed.
   SanitizerSet SanHash = LangOpts->Sanitize;
Index: lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -156,6 +156,8 @@
 LangOpts.CurrentModule.empty() ? MainFileName : LangOpts.CurrentModule;
 CodeGenOpts.setDebugInfo(codegenoptions::FullDebugInfo);
 CodeGenOpts.setDebuggerTuning(CI.getCodeGenOpts().getDebuggerTuning());
+CodeGenOpts.DebugPrefixMap

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: dblaikie, probinson.
aprantl added a project: debug-info.

As discussed on llvm-dev today, Clang currently emits redundant directories in 
DIFile entries, such as

`.file  1 "/Volumes/Data/llvm" 
"/Volumes/Data/llvm/tools/clang/test/CodeGen/debug-info-abspath.c"`

This patch looks at any common prefix between the compilation directory and the 
(absolute) file path and strips the redundant part. More importantly it leaves 
the compilation directory empty if the two paths have no common prefix.

After this patch the above entry is (assuming a compilation dir of 
"/Volumes/Data/llvm/_build"):

`.file  1 "/Volumes/Data/llvm" "tools/clang/test/CodeGen/debug-info-abspath.c"`

When building the `FileCheck` binary with debug info, this patch makes the 
build artifacts ~1kb smaller.


https://reviews.llvm.org/D55085

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-abspath.c
  test/CodeGen/debug-prefix-map.c

Index: test/CodeGen/debug-prefix-map.c
===
--- test/CodeGen/debug-prefix-map.c
+++ test/CodeGen/debug-prefix-map.c
@@ -17,18 +17,22 @@
 }
 
 // CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{/|\\5C}}"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "var/empty{{[/\\]}}{{.*}}",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "/")
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "var/empty{{[/\\]}}Inputs/stdio.h",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "/")
 // CHECK-NO-MAIN-FILE-NAME-NOT: !DIFile(filename:
 
 // CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}{{.*}}"
-// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-EVIL: !DIFile(filename: "var=empty{{[/\\]}}{{.*}}Inputs/stdio.h",
+// CHECK-EVIL-SAME:directory: "/")
 // CHECK-EVIL-NOT: !DIFile(filename:
 
 // CHECK: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK: !DIFile(filename: "var/empty{{[/\\]}}{{.*}}Inputs/stdio.h",
+// CHECK-SAME:directory: "/")
 // CHECK-NOT: !DIFile(filename:
 
-// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}", directory: "/var/empty")
-// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h", directory: "/var/empty")
+// CHECK-COMPILATION-DIR: !DIFile(filename: "{{.*}}", directory: "/var/empty")
+// CHECK-COMPILATION-DIR: !DIFile(filename: "Inputs/stdio.h", directory: "/var/empty")
 // CHECK-COMPILATION-DIR-NOT: !DIFile(filename:
Index: test/CodeGen/debug-info-abspath.c
===
--- /dev/null
+++ test/CodeGen/debug-info-abspath.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+// RUN: cp %s %t.c
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %t.c -emit-llvm -o - | FileCheck %s --check-prefix=INTREE
+void foo() {}
+
+// Since %s is an absolute path, directory should be a nonempty
+// prefix, but the CodeGen part should be part of the filename.
+
+// CHECK: DIFile(filename: "{{.*}}CodeGen{{.*}}debug-info-abspath.c"
+// CHECK-SAME:   directory: "{{.+}}")
+
+// INTREE: DIFile({{.*}}directory: "{{.+}}CodeGen{{.*}}")
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -407,13 +407,13 @@
   SourceManager &SM = CGM.getContext().getSourceManager();
   PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 
-  if (PLoc.isInvalid() || StringRef(PLoc.getFilename()).empty())
+  StringRef FileName = PLoc.getFilename();
+  if (PLoc.isInvalid() || FileName.empty())
 // If the location is not valid then use main input file.
 return getOrCreateMainFile();
 
   // Cache the results.
-  const char *fname = PLoc.getFilename();
-  auto It = DIFileCache.find(fname);
+  auto It = DIFileCache.find(FileName.data());
 
   if (It != DIFileCache.end()) {
 // Verify that the information still exists.
@@ -428,11 +428,34 @@
   if (CSKind)
 CSInfo.emplace(*CSKind, Checksum);
 
-  llvm::DIFile *F = DBuilder.createFile(
-  remapDIPath(PLoc.getFilename()), remapDIPath(getCurrentDirname()), CSInfo,
-  getSource(SM, SM.getFileID(Loc)));
+  StringRef Dir;
+  StringRef File;
+  std::string RemappedFile = remapDIPath(FileName);
+  std::string CurDir = remapDIPath(getCurrentDirname());
+  SmallString<128> DirBuf;
+  SmallString<128> FileBuf;
+  if (llvm::sys::path::is_absolute(RemappedFile)) {
+// Strip the common prefix from current directory and FileName for
+// a more space-efficient encoding.
+auto FileIt = llvm::sys::path::begin(RemappedFile);
+auto FileE = llvm::sys::path::end(Remap

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 175982.
aprantl added a comment.

Bugfix for LexicalBlockFiles + testcase updates.


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

https://reviews.llvm.org/D55085

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGen/debug-info-abspath.c
  test/CodeGen/debug-prefix-map.c
  test/Modules/module-debuginfo-prefix.m

Index: test/Modules/module-debuginfo-prefix.m
===
--- test/Modules/module-debuginfo-prefix.m
+++ test/Modules/module-debuginfo-prefix.m
@@ -20,4 +20,4 @@
 @import DebugObjC;
 #endif
 
-// CHECK: !DIFile({{.*}}"/OVERRIDE/DebugObjC.h"
+// CHECK: !DIFile(filename: "OVERRIDE/DebugObjC.h", directory: "/")
Index: test/CodeGen/debug-prefix-map.c
===
--- test/CodeGen/debug-prefix-map.c
+++ test/CodeGen/debug-prefix-map.c
@@ -17,18 +17,22 @@
 }
 
 // CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{/|\\5C}}"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "var/empty{{[/\\]}}{{.*}}",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "/")
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "var/empty{{[/\\]}}Inputs/stdio.h",
+// CHECK-NO-MAIN-FILE-NAME-SAME:directory: "/")
 // CHECK-NO-MAIN-FILE-NAME-NOT: !DIFile(filename:
 
 // CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}{{.*}}"
-// CHECK-EVIL: !DIFile(filename: "/var=empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-EVIL: !DIFile(filename: "var=empty{{[/\\]}}{{.*}}Inputs/stdio.h",
+// CHECK-EVIL-SAME:directory: "/")
 // CHECK-EVIL-NOT: !DIFile(filename:
 
 // CHECK: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK: !DIFile(filename: "var/empty{{[/\\]}}{{.*}}Inputs/stdio.h",
+// CHECK-SAME:directory: "/")
 // CHECK-NOT: !DIFile(filename:
 
-// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}", directory: "/var/empty")
-// CHECK-COMPILATION-DIR: !DIFile(filename: "/var/empty{{[/\\]}}Inputs/stdio.h", directory: "/var/empty")
+// CHECK-COMPILATION-DIR: !DIFile(filename: "{{.*}}", directory: "/var/empty")
+// CHECK-COMPILATION-DIR: !DIFile(filename: "Inputs/stdio.h", directory: "/var/empty")
 // CHECK-COMPILATION-DIR-NOT: !DIFile(filename:
Index: test/CodeGen/debug-info-abspath.c
===
--- /dev/null
+++ test/CodeGen/debug-info-abspath.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+// RUN: cp %s %t.c
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   %t.c -emit-llvm -o - | FileCheck %s --check-prefix=INTREE
+void foo() {}
+
+// Since %s is an absolute path, directory should be a nonempty
+// prefix, but the CodeGen part should be part of the filename.
+
+// CHECK: DIFile(filename: "{{.*}}CodeGen{{.*}}debug-info-abspath.c"
+// CHECK-SAME:   directory: "{{.+}}")
+
+// INTREE: DIFile({{.*}}directory: "{{.+}}CodeGen{{.*}}")
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -181,8 +181,7 @@
   SourceManager &SM = CGM.getContext().getSourceManager();
   auto *Scope = cast(LexicalBlockStack.back());
   PresumedLoc PCLoc = SM.getPresumedLoc(CurLoc);
-
-  if (PCLoc.isInvalid() || Scope->getFilename() == PCLoc.getFilename())
+  if (PCLoc.isInvalid() || Scope->getFile() == getOrCreateFile(CurLoc))
 return;
 
   if (auto *LBF = dyn_cast(Scope)) {
@@ -407,13 +406,13 @@
   SourceManager &SM = CGM.getContext().getSourceManager();
   PresumedLoc PLoc = SM.getPresumedLoc(Loc);
 
-  if (PLoc.isInvalid() || StringRef(PLoc.getFilename()).empty())
+  StringRef FileName = PLoc.getFilename();
+  if (PLoc.isInvalid() || FileName.empty())
 // If the location is not valid then use main input file.
 return getOrCreateMainFile();
 
   // Cache the results.
-  const char *fname = PLoc.getFilename();
-  auto It = DIFileCache.find(fname);
+  auto It = DIFileCache.find(FileName.data());
 
   if (It != DIFileCache.end()) {
 // Verify that the information still exists.
@@ -428,11 +427,34 @@
   if (CSKind)
 CSInfo.emplace(*CSKind, Checksum);
 
-  llvm::DIFile *F = DBuilder.createFile(
-  remapDIPath(PLoc.getFilename()), remapDIPath(getCurrentDirname()), CSInfo,
-  getSource(SM, SM.getFileID(Loc)));
+  StringRef Dir;
+  StringRef File;
+  std::string RemappedFile = remapDIPath(FileName);
+  std::string CurDir = remapDIPath(getCurrentDirname());
+  SmallString<128> DirBuf;
+  SmallString<128> FileBuf;
+  if (llvm::sys::path::is_absolute(RemappedFile)) {
+// Strip the common prefix from current directory and FileName for
+// a

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176148.
aprantl added a reviewer: davide.
aprantl added a comment.
Herald added subscribers: nhaehnle, jvesely.

Turns out that my patch had an unintended interaction with the backend 
diagnostics engine. This is an updated version of the patch that also updates 
the backend diagnostics engine to still emit the same diagnostics as before 
even with the more efficient DIFile encoding.

The backend diagnostics call back into the frontend to emit nicely formatted 
diagnostics, but my initial CFE patch broke this mechanism for files with an 
absolute path. This is now fixed by first looking up the relative path in the 
FileManager and then falling back to the absolute path, which we construct by 
joining the DIFile's directory and filename.


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

https://reviews.llvm.org/D55085

Files:
  include/llvm/IR/DiagnosticInfo.h
  lib/CodeGen/MachineLoopInfo.cpp
  lib/IR/DiagnosticInfo.cpp
  test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
  tools/clang/lib/CodeGen/CGDebugInfo.cpp
  tools/clang/lib/CodeGen/CodeGenAction.cpp
  tools/clang/test/CodeGen/debug-info-abspath.c
  tools/clang/test/CodeGen/debug-prefix-map.c
  tools/clang/test/Modules/module-debuginfo-prefix.m

Index: test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
===
--- test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
+++ test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
@@ -17,7 +17,7 @@
 !llvm.module.flags = !{!2, !3}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_OpenCL, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
-!1 = !DIFile(filename: "foo.cl", directory: "/dev/null")
+!1 = !DIFile(filename: "foo.cl", directory: ".")
 !2 = !{i32 2, !"Dwarf Version", i32 4}
 !3 = !{i32 2, !"Debug Info Version", i32 3}
 !4 = !DILocation(line: 1, column: 42, scope: !5)
Index: lib/IR/DiagnosticInfo.cpp
===
--- lib/IR/DiagnosticInfo.cpp
+++ lib/IR/DiagnosticInfo.cpp
@@ -33,9 +33,10 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
@@ -106,7 +107,7 @@
 DiagnosticLocation::DiagnosticLocation(const DebugLoc &DL) {
   if (!DL)
 return;
-  Filename = DL->getFilename();
+  File = DL->getFile();
   Line = DL->getLine();
   Column = DL->getColumn();
 }
@@ -114,17 +115,32 @@
 DiagnosticLocation::DiagnosticLocation(const DISubprogram *SP) {
   if (!SP)
 return;
-  Filename = SP->getFilename();
+  
+  File = SP->getFile();
   Line = SP->getScopeLine();
   Column = 0;
 }
 
-void DiagnosticInfoWithLocationBase::getLocation(StringRef *Filename,
- unsigned *Line,
- unsigned *Column) const {
-  *Filename = Loc.getFilename();
-  *Line = Loc.getLine();
-  *Column = Loc.getColumn();
+std::string DiagnosticLocation::getAbsolutePath() const {
+  StringRef Name = File->getFilename();
+  if (sys::path::is_absolute(Name))
+return Name;
+
+  SmallString<128> Path;
+  sys::path::append(Path, File->getDirectory(), Name);
+  return sys::path::remove_leading_dotslash(Path).str();
+}
+
+std::string DiagnosticInfoWithLocationBase::getAbsolutePath() const {
+  return Loc.getAbsolutePath();
+}
+
+void DiagnosticInfoWithLocationBase::getLocation(StringRef &RelativePath,
+ unsigned &Line,
+ unsigned &Column) const {
+  RelativePath = Loc.getRelativePath();
+  Line = Loc.getLine();
+  Column = Loc.getColumn();
 }
 
 const std::string DiagnosticInfoWithLocationBase::getLocationStr() const {
@@ -132,7 +148,7 @@
   unsigned Line = 0;
   unsigned Column = 0;
   if (isLocationAvailable())
-getLocation(&Filename, &Line, &Column);
+getLocation(Filename, Line, Column);
   return (Filename + ":" + Twine(Line) + ":" + Twine(Column)).str();
 }
 
@@ -399,7 +415,7 @@
   static void mapping(IO &io, DiagnosticLocation &DL) {
 assert(io.outputting() && "input not yet implemented");
 
-StringRef File = DL.getFilename();
+StringRef File = DL.getRelativePath();
 unsigned Line = DL.getLine();
 unsigned Col = DL.getColumn();
 
Index: lib/CodeGen/MachineLoopInfo.cpp
===
--- lib/CodeGen/MachineLoopInfo.cpp
+++ lib/CodeGen/MachineLoopInfo.cpp
@@ -19,6 +19,7 @@
 #include "llvm/CodeGen/MachineDominators.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
@@ -92,8 +93,10 @

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176152.
aprantl added a comment.

Remove debugging code accidentally left in the patch.


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

https://reviews.llvm.org/D55085

Files:
  include/llvm/IR/DiagnosticInfo.h
  lib/CodeGen/MachineLoopInfo.cpp
  lib/IR/DiagnosticInfo.cpp
  test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
  tools/clang/lib/CodeGen/CGDebugInfo.cpp
  tools/clang/lib/CodeGen/CodeGenAction.cpp
  tools/clang/test/CodeGen/debug-info-abspath.c
  tools/clang/test/CodeGen/debug-prefix-map.c
  tools/clang/test/Modules/module-debuginfo-prefix.m

Index: test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
===
--- test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
+++ test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
@@ -17,7 +17,7 @@
 !llvm.module.flags = !{!2, !3}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_OpenCL, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
-!1 = !DIFile(filename: "foo.cl", directory: "/dev/null")
+!1 = !DIFile(filename: "foo.cl", directory: ".")
 !2 = !{i32 2, !"Dwarf Version", i32 4}
 !3 = !{i32 2, !"Debug Info Version", i32 3}
 !4 = !DILocation(line: 1, column: 42, scope: !5)
Index: lib/IR/DiagnosticInfo.cpp
===
--- lib/IR/DiagnosticInfo.cpp
+++ lib/IR/DiagnosticInfo.cpp
@@ -33,9 +33,10 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
@@ -106,7 +107,7 @@
 DiagnosticLocation::DiagnosticLocation(const DebugLoc &DL) {
   if (!DL)
 return;
-  Filename = DL->getFilename();
+  File = DL->getFile();
   Line = DL->getLine();
   Column = DL->getColumn();
 }
@@ -114,17 +115,32 @@
 DiagnosticLocation::DiagnosticLocation(const DISubprogram *SP) {
   if (!SP)
 return;
-  Filename = SP->getFilename();
+  
+  File = SP->getFile();
   Line = SP->getScopeLine();
   Column = 0;
 }
 
-void DiagnosticInfoWithLocationBase::getLocation(StringRef *Filename,
- unsigned *Line,
- unsigned *Column) const {
-  *Filename = Loc.getFilename();
-  *Line = Loc.getLine();
-  *Column = Loc.getColumn();
+std::string DiagnosticLocation::getAbsolutePath() const {
+  StringRef Name = File->getFilename();
+  if (sys::path::is_absolute(Name))
+return Name;
+
+  SmallString<128> Path;
+  sys::path::append(Path, File->getDirectory(), Name);
+  return sys::path::remove_leading_dotslash(Path).str();
+}
+
+std::string DiagnosticInfoWithLocationBase::getAbsolutePath() const {
+  return Loc.getAbsolutePath();
+}
+
+void DiagnosticInfoWithLocationBase::getLocation(StringRef &RelativePath,
+ unsigned &Line,
+ unsigned &Column) const {
+  RelativePath = Loc.getRelativePath();
+  Line = Loc.getLine();
+  Column = Loc.getColumn();
 }
 
 const std::string DiagnosticInfoWithLocationBase::getLocationStr() const {
@@ -132,7 +148,7 @@
   unsigned Line = 0;
   unsigned Column = 0;
   if (isLocationAvailable())
-getLocation(&Filename, &Line, &Column);
+getLocation(Filename, Line, Column);
   return (Filename + ":" + Twine(Line) + ":" + Twine(Column)).str();
 }
 
@@ -399,7 +415,7 @@
   static void mapping(IO &io, DiagnosticLocation &DL) {
 assert(io.outputting() && "input not yet implemented");
 
-StringRef File = DL.getFilename();
+StringRef File = DL.getRelativePath();
 unsigned Line = DL.getLine();
 unsigned Col = DL.getColumn();
 
Index: include/llvm/IR/DiagnosticInfo.h
===
--- include/llvm/IR/DiagnosticInfo.h
+++ include/llvm/IR/DiagnosticInfo.h
@@ -340,7 +340,7 @@
 };
 
 class DiagnosticLocation {
-  StringRef Filename;
+  DIFile *File = nullptr;
   unsigned Line = 0;
   unsigned Column = 0;
 
@@ -349,8 +349,11 @@
   DiagnosticLocation(const DebugLoc &DL);
   DiagnosticLocation(const DISubprogram *SP);
 
-  bool isValid() const { return !Filename.empty(); }
-  StringRef getFilename() const { return Filename; }
+  bool isValid() const { return File; }
+  /// Return the full path to the file.
+  std::string getAbsolutePath() const;
+  /// Return the file name relative to the compilation directory.
+  StringRef getRelativePath() const { return File->getFilename(); }
   unsigned getLine() const { return Line; }
   unsigned getColumn() const { return Column; }
 };
@@ -375,9 +378,13 @@
   const std::string getLocationStr() const;
 
   /// Return location information for this diagnostic in three parts:
-  /// the source file name, line number and column.
-  void getLocation(St

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Adding a few more reviewers since I'm touching the backend diagnostics. The 
backend change is supposed to be NFC, but it never hurts to have more feedback.


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

https://reviews.llvm.org/D55085



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

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: lib/IR/DiagnosticInfo.cpp:39
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 

probinson wrote:
> Do we use a case-sensitive sort of include files?  I thought it was 
> insensitive.  Of course the coding standard doesn't say. :-P
I just ran clang-format without thinking about this...


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

https://reviews.llvm.org/D55085



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

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: dblaikie, davide.
aprantl added a project: debug-info.

This adds a callback to `PrintingPolicy` to allow `CGDebugInfo` to remap file 
paths according to `-fdebug-prefix-map`. Otherwise the debug info (particularly 
function names for C++ lambdas) may contain paths that should have been 
remapped in the debug info.


https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -341,6 +341,9 @@
 
   void finalize();
 
+  /// Remap a given path with the current debug prefix map
+  std::string remapDIPath(StringRef) const;
+
   /// Register VLA size expression debug node with the qualified type.
   void registerVLASizeExpression(QualType Ty, llvm::Metadata *SizeExpr) {
 SizeExprCache[Ty] = SizeExpr;
@@ -528,9 +531,6 @@
   /// Create new compile unit.
   void CreateCompileUnit();
 
-  /// Remap a given path with the current debug prefix map
-  std::string remapDIPath(StringRef) const;
-
   /// Compute the file checksum debug info for input file ID.
   Optional
   computeChecksum(FileID FID, SmallString<32> &Checksum) const;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -234,6 +234,9 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  PP.RemapFilePaths = true;
+  PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
   return PP;
 }
 
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(f

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176174.
aprantl added a comment.

fix a comment.


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

https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -341,6 +341,9 @@
 
   void finalize();
 
+  /// Remap a path using the -fdebug-prefix-map.
+  std::string remapDIPath(StringRef Path) const;
+
   /// Register VLA size expression debug node with the qualified type.
   void registerVLASizeExpression(QualType Ty, llvm::Metadata *SizeExpr) {
 SizeExprCache[Ty] = SizeExpr;
@@ -528,9 +531,6 @@
   /// Create new compile unit.
   void CreateCompileUnit();
 
-  /// Remap a given path with the current debug prefix map
-  std::string remapDIPath(StringRef) const;
-
   /// Compute the file checksum debug info for input file ID.
   Optional
   computeChecksum(FileID FID, SmallString<32> &Checksum) const;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -234,6 +234,9 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  PP.RemapFilePaths = true;
+  PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
   return PP;
 }
 
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(false), ConstantsAsWritten(false),
+SuppressImplicitBase(false), FullyQualifiedName(false),
+RemapFilePaths(false) {}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D55137#1315133 , @dblaikie wrote:

> Can't say I know much abouth the path remapping functionality - what it's 
> used for, where it's implemented in general, etc - so figure someone with 
> more of that knowledge might be best off reviewing this.


One common use-case for path remapping is a build system that builds on a 
different machine in some local directory, but wanting the debug info appear as 
if the source code were in a previously agreed-upon path so it can be debugged 
on another machine were the source code can be found in that same path.


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

https://reviews.llvm.org/D55137



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

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176194.
aprantl added a comment.

Only initialize `RemapFilePaths` when `DebugPrefixMap` is nonempty. Should be 
slightly faster.


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

https://reviews.llvm.org/D55137

Files:
  include/clang/AST/PrettyPrinter.h
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-prefix-map-lambda.cpp

Index: test/CodeGenCXX/debug-prefix-map-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCXX/debug-prefix-map-lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple \
+// RUN:   -fdebug-prefix-map=%S=/SOURCE_ROOT %s -emit-llvm -o - | FileCheck %s
+
+template  void b(T) {}
+void c() {
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"
+  b([]{});
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -341,6 +341,9 @@
 
   void finalize();
 
+  /// Remap a path using the -fdebug-prefix-map.
+  std::string remapDIPath(StringRef Path) const;
+
   /// Register VLA size expression debug node with the qualified type.
   void registerVLASizeExpression(QualType Ty, llvm::Metadata *SizeExpr) {
 SizeExprCache[Ty] = SizeExpr;
@@ -528,9 +531,6 @@
   /// Create new compile unit.
   void CreateCompileUnit();
 
-  /// Remap a given path with the current debug prefix map
-  std::string remapDIPath(StringRef) const;
-
   /// Compute the file checksum debug info for input file ID.
   Optional
   computeChecksum(FileID FID, SmallString<32> &Checksum) const;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -234,6 +234,11 @@
   if (CGM.getCodeGenOpts().EmitCodeView)
 PP.MSVCFormatting = true;
 
+  // Apply -fdebug-prefix-map.
+  if (!DebugPrefixMap.empty()) {
+PP.RemapFilePaths = true;
+PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); };
+  }
   return PP;
 }
 
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1157,9 +1157,13 @@
   PresumedLoc PLoc = D->getASTContext().getSourceManager().getPresumedLoc(
   D->getLocation());
   if (PLoc.isValid()) {
-OS << " at " << PLoc.getFilename()
-   << ':' << PLoc.getLine()
-   << ':' << PLoc.getColumn();
+OS << " at ";
+StringRef File = PLoc.getFilename();
+if (Policy.RemapFilePaths)
+  OS << Policy.remapPath(File);
+else
+  OS << File;
+OS << ':' << PLoc.getLine() << ':' << PLoc.getColumn();
   }
 }
 
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -38,21 +38,20 @@
 struct PrintingPolicy {
   /// Create a default printing policy for the specified language.
   PrintingPolicy(const LangOptions &LO)
-: Indentation(2), SuppressSpecifiers(false),
-  SuppressTagKeyword(LO.CPlusPlus),
-  IncludeTagDefinition(false), SuppressScope(false),
-  SuppressUnwrittenScope(false), SuppressInitializers(false),
-  ConstantArraySizeAsWritten(false), AnonymousTagLocations(true),
-  SuppressStrongLifetime(false), SuppressLifetimeQualifiers(false),
-  SuppressTemplateArgsInCXXConstructors(false),
-  Bool(LO.Bool), Restrict(LO.C99),
-  Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
-  UseVoidForZeroParams(!LO.CPlusPlus),
-  TerseOutput(false), PolishForDeclaration(false),
-  Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar),
-  IncludeNewlines(true), MSVCFormatting(false),
-  ConstantsAsWritten(false), SuppressImplicitBase(false),
-  FullyQualifiedName(false) { }
+  : Indentation(2), SuppressSpecifiers(false),
+SuppressTagKeyword(LO.CPlusPlus), IncludeTagDefinition(false),
+SuppressScope(false), SuppressUnwrittenScope(false),
+SuppressInitializers(false), ConstantArraySizeAsWritten(false),
+AnonymousTagLocations(true), SuppressStrongLifetime(false),
+SuppressLifetimeQualifiers(false),
+SuppressTemplateArgsInCXXConstructors(false), Bool(LO.Bool),
+Restrict(LO.C99), Alignof(LO.CPlusPlus11), UnderscoreAlignof(LO.C11),
+UseVoidForZeroParams(!LO.CPlusPlus), TerseOutput(false),
+PolishForDeclaration(false), Half(LO.Half),
+MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
+MSVCFormatting(false), ConstantsAsWritten(false),
+SuppressImplicitBase(false), FullyQualifiedName(false),
+

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: test/CodeGenCXX/debug-prefix-map-lambda.cpp:7
+  // CHECK: !DISubprogram(name: "b<(lambda at
+  // CHECK-SAME:  /SOURCE_ROOT/debug-prefix-map-lambda.cpp
+  // CHECK-SAME:  [[@LINE+1]]:{{[0-9]+}})>"

probinson wrote:
> I've never liked tests that hard-code the test file name. FileCheck lets you 
> define variables on the RUN line, is there a lit substitution that will 
> return just the basename?
Looks like there is not LIT substitution for the filename without the path. At 
least not a documented one.


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

https://reviews.llvm.org/D55137



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

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176677.
aprantl added a reviewer: ilya-biryukov.
aprantl added a comment.

Ilya, this updated revision should restore the original GCOV behavior both for 
absolute and relative paths.


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

https://reviews.llvm.org/D55085

Files:
  include/llvm/IR/DiagnosticInfo.h
  lib/IR/DiagnosticInfo.cpp
  lib/Transforms/Instrumentation/GCOVProfiling.cpp
  test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
  tools/clang/lib/CodeGen/CGDebugInfo.cpp
  tools/clang/lib/CodeGen/CodeGenAction.cpp
  tools/clang/test/CodeGen/debug-info-abspath.c
  tools/clang/test/CodeGen/debug-prefix-map.c
  tools/clang/test/Modules/module-debuginfo-prefix.m

Index: test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
===
--- test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
+++ test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
@@ -17,7 +17,7 @@
 !llvm.module.flags = !{!2, !3}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_OpenCL, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
-!1 = !DIFile(filename: "foo.cl", directory: "/dev/null")
+!1 = !DIFile(filename: "foo.cl", directory: ".")
 !2 = !{i32 2, !"Dwarf Version", i32 4}
 !3 = !{i32 2, !"Debug Info Version", i32 3}
 !4 = !DILocation(line: 1, column: 42, scope: !5)
Index: lib/Transforms/Instrumentation/GCOVProfiling.cpp
===
--- lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -180,6 +180,21 @@
   return SP->getName();
 }
 
+/// Extract a filename for a DISubprogram.
+///
+/// Prefer relative paths in the coverage notes. Clang also may split
+/// up absolute paths into a directory and filename component. When
+/// the relative path doesn't exist, reconstruct the absolute path.
+SmallString<128> getFilename(const DISubprogram *SP) {
+  SmallString<128> Path;
+  StringRef RelPath = SP->getFilename();
+  if (sys::fs::exists(RelPath))
+Path = RelPath;
+  else
+sys::path::append(Path, SP->getDirectory(), SP->getFilename());
+  return Path;
+}
+
 namespace {
   class GCOVRecord {
protected:
@@ -256,7 +271,7 @@
 }
 
private:
-StringRef Filename;
+std::string Filename;
 SmallVector Lines;
   };
 
@@ -377,8 +392,9 @@
 
 void writeOut() {
   writeBytes(FunctionTag, 4);
+  SmallString<128> Filename = getFilename(SP);
   uint32_t BlockLen = 1 + 1 + 1 + lengthOfGCOVString(getFunctionName(SP)) +
-  1 + lengthOfGCOVString(SP->getFilename()) + 1;
+  1 + lengthOfGCOVString(Filename) + 1;
   if (UseCfgChecksum)
 ++BlockLen;
   write(BlockLen);
@@ -387,7 +403,7 @@
   if (UseCfgChecksum)
 write(CfgChecksum);
   writeGCOVString(getFunctionName(SP));
-  writeGCOVString(SP->getFilename());
+  writeGCOVString(Filename);
   write(SP->getLine());
 
   // Emit count of blocks.
@@ -466,7 +482,7 @@
   if (FilterRe.empty() && ExcludeRe.empty()) {
 return true;
   }
-  const StringRef Filename = F.getSubprogram()->getFilename();
+  SmallString<128> Filename = getFilename(F.getSubprogram());
   auto It = InstrumentedFiles.find(Filename);
   if (It != InstrumentedFiles.end()) {
 return It->second;
@@ -688,7 +704,8 @@
   // Add the function line number to the lines of the entry block
   // to have a counter for the function definition.
   uint32_t Line = SP->getLine();
-  Func.getBlock(&EntryBlock).getFile(SP->getFilename()).addLine(Line);
+  auto Filename = getFilename(SP);
+  Func.getBlock(&EntryBlock).getFile(Filename).addLine(Line);
 
   for (auto &BB : F) {
 GCOVBlock &Block = Func.getBlock(&BB);
@@ -719,7 +736,7 @@
   if (SP != getDISubprogram(Loc.getScope()))
 continue;
 
-  GCOVLines &Lines = Block.getFile(SP->getFilename());
+  GCOVLines &Lines = Block.getFile(Filename);
   Lines.addLine(Loc.getLine());
 }
 Line = 0;
Index: lib/IR/DiagnosticInfo.cpp
===
--- lib/IR/DiagnosticInfo.cpp
+++ lib/IR/DiagnosticInfo.cpp
@@ -33,9 +33,10 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
@@ -106,7 +107,7 @@
 DiagnosticLocation::DiagnosticLocation(const DebugLoc &DL) {
   if (!DL)
 return;
-  Filename = DL->getFilename();
+  File = DL->getFile();
   Line = DL->getLine();
   Column = DL->getColumn();
 }
@@ -114,17 +115,36 @@
 DiagnosticLocation::DiagnosticLocation(const DISubprogram *SP) {
   if (!SP)
 return;
-  Filename = SP->getFilename();
+  
+  File 

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks, should be fixed in r348610!


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

https://reviews.llvm.org/D55085



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

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755
+}
+V = V->stripPointerCasts();
   }

This wasn't there before... why is this necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58992



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

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Adrian Prantl via Phabricator via cfe-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755
+}
+V = V->stripPointerCasts();
   }

hliao wrote:
> aprantl wrote:
> > This wasn't there before... why is this necessary?
> That's from the original line 1752.
Oh :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58992



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

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

How can this change be NFC?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59197



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

[PATCH] D54978: Move the SMT API to LLVM

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I'm afraid this broke some bots that build with `LLVM_ENABLE_MODULES=1`.

For example:

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22411/consoleFull#710926295dd1929ea-7054-4089-b7ef-4624c3781fa4

  Undefined symbols for architecture x86_64:
"llvm::errs()", referenced from:
llvm::SMTExpr::dump() const in 
liblldbDebugserverCommon.a(RNBSocket.cpp.o)
llvm::SMTSolver::dump() const in 
liblldbDebugserverCommon.a(RNBSocket.cpp.o)
llvm::SMTSort::dump() const in 
liblldbDebugserverCommon.a(RNBSocket.cpp.o)
llvm::SMTExpr::dump() const in 
liblldbDebugserverCommon.a(SocketAddress.cpp.o)
llvm::SMTSolver::dump() const in 
liblldbDebugserverCommon.a(SocketAddress.cpp.o)
llvm::SMTSort::dump() const in 
liblldbDebugserverCommon.a(SocketAddress.cpp.o)

Long story short: You can't have an LLVM_DUMP_METHOD defined inline inside of a 
module.

One way to fix this would be to move the function body of the various

  LLVM_DUMP_METHOD void dump() const { print(llvm::errs()); }

functions into .cpp files.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks!

The problem with these methods is that LLVM_DUMP_METHOD forces the function to 
be emitted even if it is not used and Clang modules without local submodule 
visibility will implicitly include all header files in a module, which will 
then cause the missing symbol error.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



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

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

It looks like this may have broken some LLDB data formatters:
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/testReport/junit/lldb-Suite/functionalities_data-formatter_data-formatter-stl_libcxx_optional/TestDataFormatterLibcxxOptional_py/
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22934/testReport/junit/lldb-Suite/functionalities_data-formatter_data-formatter-stl_libcxx_unordered/TestDataFormatterUnordered_py/


Repository:
  rL LLVM

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

https://reviews.llvm.org/D49863



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

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/test/Driver/debug-options.c:280
 // G_STANDALONE: "-debug-info-kind=standalone"
+// G_DWARF2: "-dwarf-version=2"
 // G_DWARF4: "-dwarf-version=4"

What's that for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60238



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

[PATCH] D60238: Verify that Android targets generate DWARF 4 by default.

[Adrian Prantl via Phabricator via cfe-commits]

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

Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60238



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Did anyone take the time to look at dexter and whether it would fit the bill 
here? It would be great to avoid reimplementing its functionality just to have 
something that then only works on Windows.

My position is that for the kind of very basic end-to-end testing that we do in 
this repository (set breakpoints+print variables), it should be possible to 
have one wrapper that supports multiple debuggers. I'm also fine with adding 
platform-specific tests into a subdirectory, but that should be the exception, 
not the rule. What I would like to avoid is growing a large parallel body of 
basic tests that only work on one platform.

It sounded like dexter would be the missing glue layer that supports windows 
debuggers and unix debuggers alike. If that is feasible I would prefer to 
extend dexter to support WinDbg and migrating debuginfo-tests to use dexter 
over adding new code and more complexity to debuginfo-tests.


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

https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

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

Okay, so it looks like dexter might be a replacement for the lldgb.py wrapper 
and would support gdb, lldb, and the visual studio debugger. I think it would 
be nice to migrate the bulk of the debuginfo-tests repository to that. Until 
someone implements a cdb driver in dexter, however, I suppose there is no harm 
in adding a few files that directly talk to cdb. And I'm in no position to 
estimate how much work a cdb driver would be since I'm neither familiar with 
dexter or cdb :-)


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

https://reviews.llvm.org/D54187



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

[PATCH] D58033: Add option for emitting dbg info for call sites

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: include/clang/Driver/Options.td:919
   HelpText<"Do not use jump tables for lowering switches">;
+def emit_param_entry_values : Joined<["-"], "femit-param-entry-values">,
+   Group,

I assume that this is the same -f option that GCC uses?


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

https://reviews.llvm.org/D58033



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

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:4537
+  CGM.getLangOpts().Optimize) {
+for (auto &SP : DeclCache) {
+  auto *D = SP.first;

Just looking at the type declarations in CGDebugInfo.h: Why not iterate over 
the `SPCache`  directly? Shouldn't that contain all Function declarations only?



Comment at: lib/CodeGen/CGDebugInfo.cpp:4541
+const Stmt *FuncBody = (*FD).getBody();
+for(auto Parm : FD->parameters()) {
+  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, CGM.getContext());

clang-format please



Comment at: lib/CodeGen/CGDebugInfo.cpp:4546
+if (I != ParmCache.end()) {
+  auto *DIParm = dyn_cast(I->second);
+  DIParm->setIsNotModified();

Could this be a `cast<>`?


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

https://reviews.llvm.org/D58035



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

[PATCH] D58033: Add option for emitting dbg info for call sites

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

And if we plan to enable it by default, too, perhaps not adding a driver option 
(CC1 only) is preferable, since we need to maintain compatibility with driver 
options indefinitely.


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

https://reviews.llvm.org/D58033



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

[PATCH] D58033: Add option for emitting dbg info for call site parameters

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Is there some kind of testcase?


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

https://reviews.llvm.org/D58033



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

[PATCH] D61079: Skip type units/type uniquing when we know we're only emitting the type once (vtable-based emission when triggered by a strong vtable, with -fno-standalone-debug)

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I hope I'm getting this right, but if I remember correctly, the significant 
part in this test is what is a FwdDecl as opposed to a definition. The 
identifiers no longer make it into the debug info and you should see no 
difference in the produced binaries with this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61079



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

[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

[Adrian Prantl via Phabricator via cfe-commits]

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

Could we test this by doing -dump-ast of From and To and FileCheck-ing the 
output?


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

https://reviews.llvm.org/D61140



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

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:5039
+  if (!ToOrErr)
+// FIXME: return the error?
+consumeError(ToOrErr.takeError());

We don't typically commit FIXME's into LLVM code. Why not just deal with the 
error properly from the start?



Comment at: lldb/source/Symbol/ClangASTImporter.cpp:65
 
-  if (delegate_sp)
-return delegate_sp->Import(type);
+  if (delegate_sp) {
+if (llvm::Expected ret_or_error = delegate_sp->Import(type)) {

```
 if (!delegate_sp)
  return {};
```



Comment at: lldb/source/Symbol/ClangASTImporter.cpp:68
+  return *ret_or_error;
+} else {
+  Log *log =

The `else` is redundant.



Comment at: lldb/source/Symbol/ClangASTImporter.cpp:139
+
+  llvm::consumeError(result.takeError());
+

Can you convert this to an early return instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438



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

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/include/clang/AST/ASTImporter.h:203
 /// context, or the import error.
-llvm::Expected Import_New(TypeSourceInfo *FromTSI);
-// FIXME: Remove this version.
-TypeSourceInfo *Import(TypeSourceInfo *FromTSI);
+llvm::Expected Import(TypeSourceInfo *FromTSI);
 

Wouldn't it make more sense to return `Expected`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438



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

[PATCH] D51910: [Modules] Add platform feature to requires clause

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: docs/Modules.rst:470
+*platform variant*
+  A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, 
``win32``, ``linux``, etc) is available.
 

aprantl wrote:
> Does this work with platforms+environment combinations, such as the ios 
> simulator on Darwin?
Is `iossimulator` a thing? The triple is called 
`x86_64-apple-ios-11.2.3-simulator` and the SDK calls itself `iphonesimulator`, 
so I wonder if this shouldn't be called `ios-simulator` or `iphonesimulator` 
instead.


https://reviews.llvm.org/D51910



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

[PATCH] D51910: [Modules] Add platform feature to requires clause

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/Basic/Module.cpp:101
+  // variant (2), the simulator is hardcoded as part of the platform name. Both
+  // forms above should match "iossimulator" and "ios-simulator" features.
+  if (Target.getTriple().isOSDarwin() && PlatformEnv.endswith("simulator"))

Ah.. it looks like you are fully aware of this :-)


https://reviews.llvm.org/D51910



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

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, bruno.

When a module name is specified as -fmodule-name, that module gets a
clang::Module object, but it won't actually be built or imported; it
will be textual. CGDebugInfo wouldn't detect this and them emit a
DICompileUnit that had a hash but no name and that confused both
dsymutil, LLDB, and myself.

rdar://problem/47926508


https://reviews.llvm.org/D57976

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/Modules/DebugInfo-fmodule-name.c


Index: test/Modules/DebugInfo-fmodule-name.c
===
--- /dev/null
+++ test/Modules/DebugInfo-fmodule-name.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2296,7 +2296,14 @@
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile was not specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(


Index: test/Modules/DebugInfo-fmodule-name.c
===
--- /dev/null
+++ test/Modules/DebugInfo-fmodule-name.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-format=obj -fmodule-name=F \
+// RUN: -debug-info-kind=limited -dwarf-ext-refs \
+// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -F %S/Inputs \
+// RUN: %s -S -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s
+
+#include "F/F.h"
+
+// CHECK: !DICompileUnit
+// CHECK-NOT: dwoId:
+
+// We still want the import, but no skeleton CU, since no PCM was built.
+
+// CHECK: !DIModule({{.*}}, name: "F"
+// CHECK-NOT: !DICompileUnit
+// CHECK-NOT: dwoId:
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2296,7 +2296,14 @@
   }
 
   bool IsRootModule = M ? !M->Parent : true;
-  if (CreateSkeletonCU && IsRootModule) {
+  // When a module name is specified as -fmodule-name, that module gets a
+  // clang::Module object, but it won't actually be built or imported; it will
+  // be textual.
+  if (CreateSkeletonCU && IsRootModule && Mod.getASTFile().empty())
+assert((!M || (M->Name == CGM.getLangOpts().ModuleName)) &&
+   "clang module without ASTFile was not specified by -fmodule-name");
+
+  if (CreateSkeletonCU && IsRootModule && !Mod.getASTFile().empty()) {
 // PCH files don't have a signature field in the control block,
 // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
 // We use the lower 64 bits for debug info.
@@ -2313,6 +2320,7 @@
   Signature);
 DIB.finalize();
   }
+
   llvm::DIModule *Parent =
   IsRootModule ? nullptr
: getOrCreateModuleRef(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits