[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Please upload patches with context. `arc diff` will do this for you.




Comment at: clang/docs/ControlFlowIntegrityDesign.rst:277
 
+Forward-Edge CFI for Virtual Calls by Interleaving Virtual Tables
+=

I would add this as a subsection of "Forward-Edge CFI for Virtual Calls".



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:286
+
+On the high level, the interleaving scheme consists of two steps: 1) order 
virtual tables by a pre-order 
+traversal of the class hierarchy and 2) interleave the virtual tables entry by 
entry.

On the high level -> At a high level



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:322
+This step will arrange the virtual tables for A, B, C, and D in the order of 
*vtable-of-A, vtable-of-B, vtable-of-D, vtable-of-C*.
+In this order, for any class all the compatible virtual tables will appear 
consecutively.
+

I would move this sentence to the start of the subsection because it isn't 
specific to your example and clarify that although GlobalLayoutBuilder tries to 
place compatible vtables consecutively (but doesn't always succeed because the 
Itanium ABI glues vtables together), this algorithm requires them to appear 
consecutively.



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:331
+works under this scheme because the interleaved virtual table has the property 
that for 
+each virtual funtion the distance between an entry of this function and the 
corresponding 
+address point is always the same. 

funtion -> function



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:339
+  
+  A::offset-to-top, B::offset-to-top, D::offset-to-top, C::offset-to-top, 
&A::rtti, &B::rtti, &D::rtti, &C::rtti, &A::f1, &B::f1, &D::f1, &C::f1, &B::f2, 
&D::f2, &C::f3, &D::f4
+

This layout isn't necessarily going to work with traditional RTTI because the 
`__dynamic_cast` function is allowed to assume that the rtti and offset-to-top 
fields appear at the offsets behind the address point that the ABI says that 
they will appear at. Indeed, the libcxxabi implementation makes that assumption:

https://github.com/llvm-mirror/libcxxabi/blob/master/src/private_typeinfo.cpp#L627

It's probably more something to keep in mind for the implementation, but I 
think we at least need to mention the RTTI incompatibility here.


Repository:
  rC Clang

https://reviews.llvm.org/D50372



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

[PATCH] D50724: SafeStack: Disable Darwin support

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D50724



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

[PATCH] D50743: libcxx: Mark __temp_value::__temp_value as _LIBCPP_NO_CFI.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX339797: libcxx: Mark __temp_value::__temp_value as 
_LIBCPP_NO_CFI. (authored by pcc, committed by ).
Herald added subscribers: cfe-commits, ldionne.

Changed prior to commit:
  https://reviews.llvm.org/D50743?vs=160712&id=160852#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D50743

Files:
  include/memory


Index: include/memory
===
--- include/memory
+++ include/memory
@@ -5631,8 +5631,11 @@
 _Tp &   get() { return *__addr(); }
 
 template
-__temp_value(_Alloc &__alloc, _Args&& ... __args) : __a(__alloc)
-{ _Traits::construct(__a, __addr(), _VSTD::forward<_Args>(__args)...); }
+_LIBCPP_NO_CFI
+__temp_value(_Alloc &__alloc, _Args&& ... __args) : __a(__alloc) {
+  _Traits::construct(__a, reinterpret_cast<_Tp*>(addressof(__v)),
+ _VSTD::forward<_Args>(__args)...);
+}
 
 ~__temp_value() { _Traits::destroy(__a, __addr()); }
 };


Index: include/memory
===
--- include/memory
+++ include/memory
@@ -5631,8 +5631,11 @@
 _Tp &   get() { return *__addr(); }
 
 template
-__temp_value(_Alloc &__alloc, _Args&& ... __args) : __a(__alloc)
-{ _Traits::construct(__a, __addr(), _VSTD::forward<_Args>(__args)...); }
+_LIBCPP_NO_CFI
+__temp_value(_Alloc &__alloc, _Args&& ... __args) : __a(__alloc) {
+  _Traits::construct(__a, reinterpret_cast<_Tp*>(addressof(__v)),
+ _VSTD::forward<_Args>(__args)...);
+}
 
 ~__temp_value() { _Traits::destroy(__a, __addr()); }
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM except for a few spelling/grammar nits.




Comment at: clang/docs/ControlFlowIntegrityDesign.rst:361
+it starts by allocating two work lists, one initialized with all the 
offset-to-top entries of virtual tables in the order 
+computed in the last step, one initlized with all the RTTI entries in the same 
order. 
+

initialized



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:374
+
+Then for each virtual funciton the algorithm goes through all the virtual 
tables in the previously computed order
+to collect all the relted entries into a virtual function list. 

function



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:375
+Then for each virtual funciton the algorithm goes through all the virtual 
tables in the previously computed order
+to collect all the relted entries into a virtual function list. 
+After this step, there are the following virtual function lists:

related



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:395
+
+Next, the algorithm picks the longest remaining virtual function list and 
append the whole list to the shortest work list
+until no function list is left, and pads the shorter work list so that they 
are of the same length. 

appends



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:396
+Next, the algorithm picks the longest remaining virtual function list and 
append the whole list to the shortest work list
+until no function list is left, and pads the shorter work list so that they 
are of the same length. 
+In the example, f1 list will be first added to work list 1, then f2 list will 
be added 

lists are left


https://reviews.llvm.org/D50372



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: rnk.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51049

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/addrsig.c


Index: clang/test/Driver/addrsig.c
===
--- clang/test/Driver/addrsig.c
+++ clang/test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 
| FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig 
-c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | 
FileCheck -check-prefix=NO-ADDRSIG %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4852,7 +4852,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 


Index: clang/test/Driver/addrsig.c
===
--- clang/test/Driver/addrsig.c
+++ clang/test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4852,7 +4852,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

With ELF it was around 0.1% for a self-host of Clang. I also checked the 
overhead with COFF for Chromium's base_unittests and it was around 0.2% without 
debug info. I think that's small enough that we can turn this on by default.


Repository:
  rL LLVM

https://reviews.llvm.org/D51049



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

[PATCH] D47050: MC: Change the streamer ctors to take an object writer instead of a stream. NFCI.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332749: MC: Change the streamer ctors to take an object 
writer instead of a stream. (authored by pcc, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47050?vs=147430&id=147564#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47050

Files:
  tools/driver/cc1as_main.cpp


Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -29,6 +29,7 @@
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
+#include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
@@ -426,11 +427,12 @@
 MCTargetOptions MCOptions;
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
+std::unique_ptr OW = MAB->createObjectWriter(*Out);
 
 Triple T(Opts.Triple);
 Str.reset(TheTarget->createMCObjectStreamer(
-T, Ctx, std::move(MAB), *Out, std::move(CE), *STI, Opts.RelaxAll,
-Opts.IncrementalLinkerCompatible,
+T, Ctx, std::move(MAB), std::move(OW), std::move(CE), *STI,
+Opts.RelaxAll, Opts.IncrementalLinkerCompatible,
 /*DWARFMustBeAtTheEnd*/ true));
 Str.get()->InitSections(Opts.NoExecStack);
   }


Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -29,6 +29,7 @@
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
+#include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
@@ -426,11 +427,12 @@
 MCTargetOptions MCOptions;
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
+std::unique_ptr OW = MAB->createObjectWriter(*Out);
 
 Triple T(Opts.Triple);
 Str.reset(TheTarget->createMCObjectStreamer(
-T, Ctx, std::move(MAB), *Out, std::move(CE), *STI, Opts.RelaxAll,
-Opts.IncrementalLinkerCompatible,
+T, Ctx, std::move(MAB), std::move(OW), std::move(CE), *STI,
+Opts.RelaxAll, Opts.IncrementalLinkerCompatible,
 /*DWARFMustBeAtTheEnd*/ true));
 Str.get()->InitSections(Opts.NoExecStack);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47093: CodeGen, Driver: Start using direct split dwarf emission in clang.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: dblaikie, echristo.
Herald added a subscriber: JDevlieghere.

Fixes PR37466.

Depends on https://reviews.llvm.org/D47091


https://reviews.llvm.org/D47093

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/split-debug-filename.c
  clang/test/Driver/split-debug.c
  clang/test/Driver/split-debug.s
  clang/test/Misc/cc1as-split-dwarf.s
  clang/tools/driver/cc1as_main.cpp

Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -97,6 +97,7 @@
   llvm::DebugCompressionType CompressDebugSections =
   llvm::DebugCompressionType::None;
   std::string MainFileName;
+  std::string SplitDwarfFile;
 
   /// @}
   /// @name Frontend Options
@@ -247,6 +248,7 @@
   }
   Opts.LLVMArgs = Args.getAllArgValues(OPT_mllvm);
   Opts.OutputPath = Args.getLastArgValue(OPT_o);
+  Opts.SplitDwarfFile = Args.getLastArgValue(OPT_split_dwarf_file);
   if (Arg *A = Args.getLastArg(OPT_filetype)) {
 StringRef Name = A->getValue();
 unsigned OutputType = StringSwitch(Name)
@@ -282,22 +284,17 @@
 }
 
 static std::unique_ptr
-getOutputStream(AssemblerInvocation &Opts, DiagnosticsEngine &Diags,
-bool Binary) {
-  if (Opts.OutputPath.empty())
-Opts.OutputPath = "-";
-
+getOutputStream(StringRef Path, DiagnosticsEngine &Diags, bool Binary) {
   // Make sure that the Out file gets unlinked from the disk if we get a
   // SIGINT.
-  if (Opts.OutputPath != "-")
-sys::RemoveFileOnSignal(Opts.OutputPath);
+  if (Path != "-")
+sys::RemoveFileOnSignal(Path);
 
   std::error_code EC;
   auto Out = llvm::make_unique(
-  Opts.OutputPath, EC, (Binary ? sys::fs::F_None : sys::fs::F_Text));
+  Path, EC, (Binary ? sys::fs::F_None : sys::fs::F_Text));
   if (EC) {
-Diags.Report(diag::err_fe_unable_to_open_output) << Opts.OutputPath
- << EC.message();
+Diags.Report(diag::err_fe_unable_to_open_output) << Path << EC.message();
 return nullptr;
   }
 
@@ -342,9 +339,15 @@
   MAI->setRelaxELFRelocations(Opts.RelaxELFRelocations);
 
   bool IsBinary = Opts.OutputType == AssemblerInvocation::FT_Obj;
-  std::unique_ptr FDOS = getOutputStream(Opts, Diags, IsBinary);
+  if (Opts.OutputPath.empty())
+Opts.OutputPath = "-";
+  std::unique_ptr FDOS =
+  getOutputStream(Opts.OutputPath, Diags, IsBinary);
   if (!FDOS)
 return true;
+  std::unique_ptr DwoOS;
+  if (!Opts.SplitDwarfFile.empty())
+DwoOS = getOutputStream(Opts.SplitDwarfFile, Diags, IsBinary);
 
   // FIXME: This is not pretty. MCContext has a ptr to MCObjectFileInfo and
   // MCObjectFileInfo needs a MCContext reference in order to initialize itself.
@@ -427,7 +430,9 @@
 MCTargetOptions MCOptions;
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
-std::unique_ptr OW = MAB->createObjectWriter(*Out);
+std::unique_ptr OW =
+DwoOS ? MAB->createDwoObjectWriter(*Out, *DwoOS)
+  : MAB->createObjectWriter(*Out);
 
 Triple T(Opts.Triple);
 Str.reset(TheTarget->createMCObjectStreamer(
@@ -476,8 +481,12 @@
   FDOS.reset();
 
   // Delete output file if there were errors.
-  if (Failed && Opts.OutputPath != "-")
-sys::fs::remove(Opts.OutputPath);
+  if (Failed) {
+if (Opts.OutputPath != "-")
+  sys::fs::remove(Opts.OutputPath);
+if (!Opts.SplitDwarfFile.empty() && Opts.SplitDwarfFile != "-")
+  sys::fs::remove(Opts.SplitDwarfFile);
+  }
 
   return Failed;
 }
Index: clang/test/Misc/cc1as-split-dwarf.s
===
--- /dev/null
+++ clang/test/Misc/cc1as-split-dwarf.s
@@ -0,0 +1,25 @@
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu %s -filetype obj -o %t1 -split-dwarf-file %t2
+// RUN: llvm-objdump -s %t1 | FileCheck --check-prefix=O %s
+// RUN: llvm-objdump -s %t2 | FileCheck --check-prefix=DWO %s
+
+// O-NOT: Contents of section
+// O: Contents of section .strtab:
+// O-NOT: Contents of section
+// O: Contents of section .text:
+// O-NEXT:  c3
+// O-NEXT: Contents of section .symtab:
+// O-NOT: Contents of section
+.globl main
+main:
+.Ltmp1:
+ret
+.Ltmp2:
+
+// DWO-NOT: Contents of section
+// DWO: Contents of section .strtab:
+// DWO-NOT: Contents of section
+// DWO: Contents of section .foo.dwo:
+// DWO-NEXT:  0100
+// DWO-NOT: Contents of section
+.section .foo.dwo
+.long .Ltmp2-.Ltmp1
Index: clang/test/Driver/split-debug.s
===
--- clang/test/Driver/split-debug.s
+++ clang/test/Driver/split-debug.s
@@ -3,8 +3,7 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 //
-// CHECK-ACTIONS: objcopy{{.*}}--ext

[PATCH] D46472: [HIP] Support offloading by linker script

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp:1371-1388
+  // machines.
+  LksStream << "/*\n";
+  LksStream << "   HIP Offload Linker Script\n";
+  LksStream << " *** Automatically generated by Clang ***\n";
+  LksStream << "*/\n";
+  LksStream << "TARGET(binary)\n";
+  LksStream << "INPUT(" << BundleFileName << ")\n";

tra wrote:
> Using this linker script may present a problem.
> 
> INSERT BEFORE is not going to work with ld.gold.
> https://sourceware.org/bugzilla/show_bug.cgi?id=15373
> 
> LLD also does not handle it particularly well -- INSERT BEFORE can only be 
> used to override explicitly specified external linker script and virtually 
> nobody uses linker scripts with LLD.
> See tests in https://reviews.llvm.org/D44380
> 
If you're just trying to embed a file it may be better to use MC to write an 
ELF with something like:
```
.section .hip_fatbin
.align 16
.globl __hip_fatbin
__hip_fatbin:
.incbin "BundleFileName"
```
and add that to the link.


Repository:
  rL LLVM

https://reviews.llvm.org/D46472



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

[PATCH] D47089: CodeGen: Add a dwo output file argument to addPassesToEmitFile and hook it up to dwo output.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332881: CodeGen: Add a dwo output file argument to 
addPassesToEmitFile and hook it up… (authored by pcc, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47089?vs=147598&id=147852#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47089

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -718,7 +718,7 @@
   if (CodeGenOpts.OptimizationLevel > 0)
 CodeGenPasses.add(createObjCARCContractPass());
 
-  if (TM->addPassesToEmitFile(CodeGenPasses, OS, CGFT,
+  if (TM->addPassesToEmitFile(CodeGenPasses, OS, nullptr, CGFT,
   /*DisableVerify=*/!CodeGenOpts.VerifyModule)) {
 Diags.Report(diag::err_fe_unable_to_interface_with_target);
 return false;


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -718,7 +718,7 @@
   if (CodeGenOpts.OptimizationLevel > 0)
 CodeGenPasses.add(createObjCARCContractPass());
 
-  if (TM->addPassesToEmitFile(CodeGenPasses, OS, CGFT,
+  if (TM->addPassesToEmitFile(CodeGenPasses, OS, nullptr, CGFT,
   /*DisableVerify=*/!CodeGenOpts.VerifyModule)) {
 Diags.Report(diag::err_fe_unable_to_interface_with_target);
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47093: CodeGen, Driver: Start using direct split dwarf emission in clang.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332885: CodeGen, Driver: Start using direct split dwarf 
emission in clang. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47093?vs=147609&id=147858#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47093

Files:
  include/clang/Driver/CC1Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGen/split-debug-filename.c
  test/Driver/split-debug.c
  test/Driver/split-debug.s
  test/Misc/cc1as-split-dwarf.s
  tools/driver/cc1as_main.cpp

Index: test/CodeGen/split-debug-filename.c
===
--- test/CodeGen/split-debug-filename.c
+++ test/CodeGen/split-debug-filename.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -debug-info-kind=limited -split-dwarf-file foo.dwo -S -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -debug-info-kind=limited -enable-split-dwarf -split-dwarf-file foo.dwo -S -emit-llvm -o - %s | FileCheck --check-prefix=VANILLA %s
+// RUN: %clang_cc1 -debug-info-kind=limited -enable-split-dwarf -split-dwarf-file %t.dwo -emit-obj -o - %s | llvm-objdump -section-headers - | FileCheck --check-prefix=O %s
+// RUN: llvm-objdump -section-headers %t.dwo | FileCheck --check-prefix=DWO %s
+
 int main (void) {
   return 0;
 }
@@ -10,3 +13,6 @@
 // Testing to ensure that the dwo name is not output into the compile unit if
 // it's for vanilla split-dwarf rather than split-dwarf for implicit modules.
 // VANILLA-NOT: splitDebugFilename
+
+// O-NOT: .dwo
+// DWO: .dwo
Index: test/Driver/split-debug.s
===
--- test/Driver/split-debug.s
+++ test/Driver/split-debug.s
@@ -3,8 +3,7 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 //
-// CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
-// CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
+// CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo"
 
 
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -3,8 +3,7 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 //
-// CHECK-ACTIONS: objcopy{{.*}}--extract-dwo{{.*}}"split-debug.dwo"
-// CHECK-ACTIONS: objcopy{{.*}}--strip-dwo{{.*}}"split-debug.o"
+// CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo"
 
 
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
Index: test/Misc/cc1as-split-dwarf.s
===
--- test/Misc/cc1as-split-dwarf.s
+++ test/Misc/cc1as-split-dwarf.s
@@ -0,0 +1,25 @@
+// RUN: %clang -cc1as -triple x86_64-pc-linux-gnu %s -filetype obj -o %t1 -split-dwarf-file %t2
+// RUN: llvm-objdump -s %t1 | FileCheck --check-prefix=O %s
+// RUN: llvm-objdump -s %t2 | FileCheck --check-prefix=DWO %s
+
+// O-NOT: Contents of section
+// O: Contents of section .strtab:
+// O-NOT: Contents of section
+// O: Contents of section .text:
+// O-NEXT:  c3
+// O-NEXT: Contents of section .symtab:
+// O-NOT: Contents of section
+.globl main
+main:
+.Ltmp1:
+ret
+.Ltmp2:
+
+// DWO-NOT: Contents of section
+// DWO: Contents of section .strtab:
+// DWO-NOT: Contents of section
+// DWO: Contents of section .foo.dwo:
+// DWO-NEXT:  0100
+// DWO-NOT: Contents of section
+.section .foo.dwo
+.long .Ltmp2-.Ltmp1
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -104,7 +104,17 @@
   ///
   /// \return True on success.
   bool AddEmitPasses(legacy::PassManager &CodeGenPasses, BackendAction Action,
- raw_pwrite_stream &OS);
+ raw_pwrite_stream &OS, raw_pwrite_stream *DwoOS);
+
+  std::unique_ptr openOutputFile(StringRef Path) {
+std::error_code EC;
+auto F = make_unique(Path, EC, llvm::sys::fs::F_None);
+if (EC) {
+  Diags.Report(diag::err_fe_unable_to_open_output) << Path << EC.message();
+  F.reset();
+}
+return F;
+  }
 
 public:
   EmitAssemblyHelper(DiagnosticsEngine &_Diags,
@@ -701,7 +711,8 @@
 
 bool EmitAssemblyHelper::AddEmitPasses(legacy::PassManager &CodeGenPasses,
BackendAction Action,
-   raw_pwrite_stream &OS) {
+   raw_pwrite_stream &OS,
+   raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
   llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
@@ -718,7 +729,7 @@
   if (CodeGenOpts.OptimizationLevel > 0)
 CodeGe

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

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

There were a bunch of them but the last one was https://reviews.llvm.org/D47093 
which has already landed :)

Probably all that needs to happen on this change is to replace the isLinux() 
check with isELF().


https://reviews.llvm.org/D46791



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

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: rnk, rsmith.

Codebases that need to be compatible with the Microsoft ABI can enable
this warning to avoid issues caused by the lack of a fixed ABI for
incomplete member pointers.


https://reviews.llvm.org/D47503

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/warn-incomplete-member-pointers.cpp


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*foo; // expected-warning {{member pointer has incomplete base 
type 'S'}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2448,6 +2448,12 @@
 return QualType();
   }
 
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*foo; // expected-warning {{member pointer has incomplete base type 'S'}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2448,6 +2448,12 @@
 return QualType();
   }
 
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 149000.
pcc added a comment.

- Add some negative tests


https://reviews.llvm.org/D47503

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/warn-incomplete-member-pointers.cpp


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*foo; // expected-warning {{member pointer has incomplete base 
type 'S'}}
+
+struct S2 {
+  typedef int S2::*foo;
+};
+
+template 
+struct S3 {
+  typedef int T::*foo;
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2448,6 +2448,12 @@
 return QualType();
   }
 
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*foo; // expected-warning {{member pointer has incomplete base type 'S'}}
+
+struct S2 {
+  typedef int S2::*foo;
+};
+
+template 
+struct S3 {
+  typedef int T::*foo;
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2448,6 +2448,12 @@
 return QualType();
   }
 
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 149001.
pcc added a comment.

- One more negative test


https://reviews.llvm.org/D47503

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/warn-incomplete-member-pointers.cpp


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*foo; // expected-warning {{member pointer has incomplete base 
type 'S'}}
+
+struct S2 {
+  typedef int S2::*foo;
+};
+typedef int S2::*bar;
+
+template 
+struct S3 {
+  typedef int T::*foo;
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2448,6 +2448,12 @@
 return QualType();
   }
 
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*foo; // expected-warning {{member pointer has incomplete base type 'S'}}
+
+struct S2 {
+  typedef int S2::*foo;
+};
+typedef int S2::*bar;
+
+template 
+struct S3 {
+  typedef int T::*foo;
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2448,6 +2448,12 @@
 return QualType();
   }
 
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:2451-2455
+  // FIXME: This will cause errors if template instantiation fails.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete, Loc) &&
+  !Class->isDependentType() &&
+  !Class->getAsCXXRecordDecl()->isBeingDefined())
+RequireCompleteType(Loc, Class, diag::warn_memptr_incomplete);

rsmith wrote:
> This doesn't seem right. Calling `RequireCompleteType` will trigger 
> instantiation of the type if it's templated, which can affect the validity 
> (and rarely, the meaning) of the program. Also, passing a warning diagnostic 
> into `RequireCompleteType` doesn't actually work -- there are cases where it 
> will disregard your diagnostic and substitute one of its own, which will be 
> an error.
You're right, but I couldn't see a way of testing whether a type is complete 
without triggering those side effects. It does look like we can at least avoid 
some of them with `isCompleteType`, though. I guess the best we can do is to 
move the diagnostic into `RequireCompleteTypeImpl` and make it conditional on 
`isCompleteType` as @rnk suggested.


https://reviews.llvm.org/D47503



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

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 149017.
pcc added a comment.

- Move the warning to RequireCompleteTypeImpl


https://reviews.llvm.org/D47503

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/warn-incomplete-member-pointers.cpp


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S;
+typedef int S::*t;
+t foo; // expected-warning {{member pointer has incomplete base type 'S'}}
+
+struct S2 {
+  int S2::*foo;
+};
+int S2::*bar;
+
+template 
+struct S3 {
+  int T::*foo;
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7585,14 +7585,23 @@
   //  assert(!T->isDependentType() &&
   // "Can't ask whether a dependent type is complete");
 
-  // We lock in the inheritance model once somebody has asked us to ensure
-  // that a pointer-to-member type is complete.
-  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-if (const MemberPointerType *MPTy = T->getAs()) {
-  if (!MPTy->getClass()->isDependentType()) {
+  if (const MemberPointerType *MPTy = T->getAs()) {
+if (!MPTy->getClass()->isDependentType()) {
+  // We lock in the inheritance model once somebody has asked us to ensure
+  // that a pointer-to-member type is complete.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
 (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
 assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
   }
+
+  // We only test for this warning if enabled because testing whether a 
type
+  // is complete has side effects.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete,
+  Loc) &&
+  !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&
+  !isCompleteType(Loc, QualType(MPTy->getClass(), 0)))
+Diag(Loc, diag::warn_memptr_incomplete)
+<< QualType(MPTy->getClass(), 0);
 }
   }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6476,6 +6476,9 @@
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">,
+  InGroup>, DefaultIgnore;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup;


Index: clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-incomplete-member-pointers.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -Wincomplete-member-pointers %s
+
+struct S;
+typedef int S::*t;
+t foo; // expected-warning {{member pointer has incomplete base type 'S'}}
+
+struct S2 {
+  int S2::*foo;
+};
+int S2::*bar;
+
+template 
+struct S3 {
+  int T::*foo;
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7585,14 +7585,23 @@
   //  assert(!T->isDependentType() &&
   // "Can't ask whether a dependent type is complete");
 
-  // We lock in the inheritance model once somebody has asked us to ensure
-  // that a pointer-to-member type is complete.
-  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-if (const MemberPointerType *MPTy = T->getAs()) {
-  if (!MPTy->getClass()->isDependentType()) {
+  if (const MemberPointerType *MPTy = T->getAs()) {
+if (!MPTy->getClass()->isDependentType()) {
+  // We lock in the inheritance model once somebody has asked us to ensure
+  // that a pointer-to-member type is complete.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
 (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
 assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
   }
+
+  // We only test for this warning if enabled because testing whether a type
+  // is complete has side effects.
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete,
+  Loc) &&
+  !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&
+  !isCompleteType(Loc, QualType(MPTy->getClass(), 0)))
+Diag(Loc, diag::warn_memptr_incomplete)
+<< QualType(

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7599-7604
+  if (!Context.getDiagnostics().isIgnored(diag::warn_memptr_incomplete,
+  Loc) &&
+  !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&
+  !isCompleteType(Loc, QualType(MPTy->getClass(), 0)))
+Diag(Loc, diag::warn_memptr_incomplete)
+<< QualType(MPTy->getClass(), 0);

rsmith wrote:
> It's not reasonable to call `isCompleteType` here; this is non-conforming 
> behavior, and it's not reasonable for a warning flag to change the semantics 
> or interpretation of the program. (We actually rely on this in several ways 
> -- `-Weverything` is not supposed to change the validity of programs, and 
> reuse of implicit modules with different warning flags relies on the property 
> that the compiler behavior is as if we compile with `-Weverything` and then 
> filter the warnings after the fact.)
> 
> It seems fine to do this in the MS ABI case, since we will attempt to 
> complete the type anyway in that mode. If you want to apply this more 
> generally, I think there are two options: either add a `-f` flag to carry 
> this semantic change, or figure out whether the type is completable without 
> actually completing it (for example, check whether it's a templated class 
> whose pattern is a definition).
That all seems reasonable. I think the right thing to do here is to add a 
`-fcomplete-member-pointers` flag then -- couple of reasons:
- a user of this feature would probably expect to see diagnostics in the case 
where `RequireCompleteType` would fail in MS mode;
- I'm planning to use the extra semantic information that would result from 
turning this on in IRgen when a new `-fsanitize=cfi-*` flag is enabled, which 
would mean that we would actually need to call `RequireCompleteType` at Sema 
time. I was thinking about tying the extra `RequireCompleteType` calls to the 
new `-fsanitize=cfi-*` flag, but that seems somewhat questionable as well (one 
of the main reasons is that it would be part of the `-fsanitize=cfi` group, and 
I don't want to make `-fsanitize=cfi` non-conforming), so we can probably get 
away with just a recommendation that `-fcomplete-member-pointers` is used with 
`-fsanitize=cfi`.


https://reviews.llvm.org/D47503



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

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 149029.
pcc added a comment.

- Implement as -f flag


https://reviews.llvm.org/D47503

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Driver/complete-member-pointers.cpp
  clang/test/SemaCXX/complete-member-pointers.cpp

Index: clang/test/SemaCXX/complete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/complete-member-pointers.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fcomplete-member-pointers %s
+
+struct S; // expected-note {{forward declaration of 'S'}}
+typedef int S::*t;
+t foo; // expected-error {{member pointer has incomplete base type 'S'}}
+
+struct S2 {
+  int S2::*foo;
+};
+int S2::*bar;
+
+template 
+struct S3 {
+  int T::*foo;
+};
Index: clang/test/Driver/complete-member-pointers.cpp
===
--- /dev/null
+++ clang/test/Driver/complete-member-pointers.cpp
@@ -0,0 +1,7 @@
+// RUN: %clangxx -### -c %s -o %t.o -target x86_64-unknown-linux 2>&1 | FileCheck --check-prefix=NOFLAG %s
+// RUN: %clangxx -### -c %s -o %t.o -target x86_64-unknown-linux -fcomplete-member-pointers 2>&1 | FileCheck %s
+// RUN: %clangxx -### -c %s -o %t.o -target x86_64-unknown-linux -fcomplete-member-pointers -fno-complete-member-pointers 2>&1 | FileCheck --check-prefix=NOFLAG %s
+// RUN: %clang_cl -### /c %s /Fo%t.o -target x86_64-pc-win32 -fcomplete-member-pointers 2>&1 | FileCheck %s
+
+// CHECK: "-fcomplete-member-pointers"
+// NOFLAG-NOT: "-fcomplete-member-pointers"
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7585,11 +7585,17 @@
   //  assert(!T->isDependentType() &&
   // "Can't ask whether a dependent type is complete");
 
-  // We lock in the inheritance model once somebody has asked us to ensure
-  // that a pointer-to-member type is complete.
-  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-if (const MemberPointerType *MPTy = T->getAs()) {
-  if (!MPTy->getClass()->isDependentType()) {
+  if (const MemberPointerType *MPTy = T->getAs()) {
+if (!MPTy->getClass()->isDependentType()) {
+  if (getLangOpts().CompleteMemberPointers &&
+  !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&
+  RequireCompleteType(Loc, QualType(MPTy->getClass(), 0),
+  diag::err_memptr_incomplete))
+return true;
+
+  // We lock in the inheritance model once somebody has asked us to ensure
+  // that a pointer-to-member type is complete.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
 (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
 assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
   }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2754,6 +2754,8 @@
   << A->getAsString(Args) << A->getValue();
 }
   }
+
+  Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4784,6 +4784,10 @@
   CmdArgs.push_back("-fforce-enable-int128");
   }
 
+  if (Args.hasFlag(options::OPT_fcomplete_member_pointers,
+   options::OPT_fno_complete_member_pointers, false))
+CmdArgs.push_back("-fcomplete-member-pointers");
+
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
   Output.getType() == types::TY_Object &&
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -776,6 +776,12 @@
 def fparse_all_comments : Flag<["-"], "fparse-all-comments">, Group, Flags<[CC1Option]>;
 def fcommon : Flag<["-"], "fcommon">, Group;
 def fcompile_resource_EQ : Joined<["-"], "fcompile-resource=">, Group;
+def fcomplete_member_pointers : Flag<["-"], "fcomplete-member-pointers">, Group,
+   Flags<[CoreOption, CC1Option]>,
+   HelpText<"Require member pointer base types to be complete if they would be significant under the Microsoft ABI">;
+def fno_complete_member_pointers : Flag<["-"], "fno-complete-member-pointers">, Group,
+   Flags<[CoreOption]>,
+   HelpText<"Do not require member point

[PATCH] D47503: Sema: Add a flag for rejecting member pointers with incomplete base types.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333498: Sema: Add a flag for rejecting member pointers with 
incomplete base types. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47503?vs=149029&id=149031#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47503

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaType.cpp
  test/Driver/complete-member-pointers.cpp
  test/SemaCXX/complete-member-pointers.cpp

Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4784,6 +4784,10 @@
   CmdArgs.push_back("-fforce-enable-int128");
   }
 
+  if (Args.hasFlag(options::OPT_fcomplete_member_pointers,
+   options::OPT_fno_complete_member_pointers, false))
+CmdArgs.push_back("-fcomplete-member-pointers");
+
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
   Output.getType() == types::TY_Object &&
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7585,11 +7585,17 @@
   //  assert(!T->isDependentType() &&
   // "Can't ask whether a dependent type is complete");
 
-  // We lock in the inheritance model once somebody has asked us to ensure
-  // that a pointer-to-member type is complete.
-  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-if (const MemberPointerType *MPTy = T->getAs()) {
-  if (!MPTy->getClass()->isDependentType()) {
+  if (const MemberPointerType *MPTy = T->getAs()) {
+if (!MPTy->getClass()->isDependentType()) {
+  if (getLangOpts().CompleteMemberPointers &&
+  !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&
+  RequireCompleteType(Loc, QualType(MPTy->getClass(), 0),
+  diag::err_memptr_incomplete))
+return true;
+
+  // We lock in the inheritance model once somebody has asked us to ensure
+  // that a pointer-to-member type is complete.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
 (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
 assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
   }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2754,6 +2754,8 @@
   << A->getAsString(Args) << A->getValue();
 }
   }
+
+  Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -782,6 +782,12 @@
 def fparse_all_comments : Flag<["-"], "fparse-all-comments">, Group, Flags<[CC1Option]>;
 def fcommon : Flag<["-"], "fcommon">, Group;
 def fcompile_resource_EQ : Joined<["-"], "fcompile-resource=">, Group;
+def fcomplete_member_pointers : Flag<["-"], "fcomplete-member-pointers">, Group,
+   Flags<[CoreOption, CC1Option]>,
+   HelpText<"Require member pointer base types to be complete if they would be significant under the Microsoft ABI">;
+def fno_complete_member_pointers : Flag<["-"], "fno-complete-member-pointers">, Group,
+   Flags<[CoreOption]>,
+   HelpText<"Do not require member pointer base types to be complete if they would be significant under the Microsoft ABI">;
 def fconstant_cfstrings : Flag<["-"], "fconstant-cfstrings">, Group;
 def fconstant_string_class_EQ : Joined<["-"], "fconstant-string-class=">, Group;
 def fconstexpr_depth_EQ : Joined<["-"], "fconstexpr-depth=">, Group;
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -247,6 +247,10 @@
 LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file for OpenCL")
 BENIGN_LANGOPT(DelayedTemplateParsing , 1, 0, "delayed template parsing")
 LANGOPT(BlocksRuntimeOptional , 1, 0, "optional blocks runtime")
+LANGOPT(
+CompleteMemberPointers, 1, 0,
+"Require member pointer base types to be complete at the point where the "
+"type's inheritance model would be determined under the Microsoft ABI")
 
 ENUM_LANGOPT(GC, GCMode, 2, NonGC, "Objective-C Garbage Collection mode")
 ENUM_LANGOPT(ValueVisibilityMode, Visibility, 3, DefaultVisibility,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===

[PATCH] D38210: [ubsan] Port the function sanitizer to C

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Wouldn't we get false positives if there is an indirect call in C++ code that 
calls into C code (or vice versa)?

I think I'd prefer it if we came up with a precise encoding of function types 
that was independent of RTTI, and use it in all languages. One possibility 
would be to represent each function type with an object of size 1 whose name 
contains the mangled function type, and use its address as the identity of the 
function type.


https://reviews.llvm.org/D38210



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

[PATCH] D38908: Do not link clang_rt.cfi on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/test/Driver/sanitizer-ld.c:605
-// CHECK-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CFI-ANDROID-NOT: __cfi_check

Why was this passing before?


https://reviews.llvm.org/D38908



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

[PATCH] D38908: Do not link clang_rt.cfi on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D38908



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

[PATCH] D38913: [ubsan] Don't emit function signatures for virtual methods

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM

Small nit on the first line of the commit message: it should probably mention 
non-static member functions, rather than virtual methods.


https://reviews.llvm.org/D38913



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340552: Driver: Enable address-significance tables by 
default when targeting COFF. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51049?vs=161763&id=162232#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51049

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/addrsig.c


Index: test/Driver/addrsig.c
===
--- test/Driver/addrsig.c
+++ test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 
| FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig 
-c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | 
FileCheck -check-prefix=NO-ADDRSIG %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4857,7 +4857,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 


Index: test/Driver/addrsig.c
===
--- test/Driver/addrsig.c
+++ test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4857,7 +4857,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Thanks, I'll take a look.


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

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

I received another report of breakage so I reverted in 
https://reviews.llvm.org/rC340579.


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

https://reviews.llvm.org/D51199 fixes the above breakage as well as 
crbug.com/877235. Once it lands, I'll reland this change.


Repository:
  rC Clang

https://reviews.llvm.org/D51049



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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340649: Reland r340552, "Driver: Enable 
address-significance tables by default when… (authored by pcc, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51049

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/addrsig.c


Index: test/Driver/addrsig.c
===
--- test/Driver/addrsig.c
+++ test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck 
-check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 
| FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig 
-c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | 
FileCheck -check-prefix=NO-ADDRSIG %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4857,7 +4857,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 


Index: test/Driver/addrsig.c
===
--- test/Driver/addrsig.c
+++ test/Driver/addrsig.c
@@ -1,4 +1,5 @@
 // RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-pc-win32 -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
 // RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4857,7 +4857,8 @@
   }
 
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
-   getToolChain().getTriple().isOSBinFormatELF() &&
+   (getToolChain().getTriple().isOSBinFormatELF() ||
+getToolChain().getTriple().isOSBinFormatCOFF()) &&
getToolChain().useIntegratedAs()))
 CmdArgs.push_back("-faddrsig");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45588: Start reserving x18 by default on Android targets.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340889: Start reserving x18 by default on Android targets. 
(authored by pcc, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45588?vs=142448&id=163003#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45588

Files:
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -575,6 +575,9 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -target arm64-unknown-ios -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
+// RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-unknown-linux-android -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
 // CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18-NOT: error:
 
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -575,6 +575,9 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -target arm64-unknown-ios -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
+// RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-unknown-linux-android -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
 // CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18-NOT: error:
 
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51905: Front-end of the implementation of the interleaving algorithm

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a subscriber: cfe-commits.
pcc added a reviewer: rsmith.
pcc added inline comments.



Comment at: clang/lib/CodeGen/CGCXXABI.h:53
+  // interleaved layout is decided.
+  bool EnableVTableInterleaving;
+

Why does this need to be stored separately on the CGCXXABI? Can't we always 
access it via the CodeGenOptions on CGM?



Comment at: clang/lib/CodeGen/CGClass.cpp:2769
 llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad(
 const CXXRecordDecl *RD, llvm::Value *VTable, uint64_t VTableByteOffset) {
   SanitizerScope SanScope(this);

Do you need this function as well as the one you're adding? Can't you change 
the code in MicrosoftCXXABI.cpp to form a constant?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5190
 bool CodeGenModule::NeedAllVtablesTypeId() const {
-  // Returns true if at least one of vtable-based CFI checkers is enabled and
-  // is not in the trapping mode.
+  // Returns true if vtable interleaving is disabled and at least one of
+  // vtable-based CFI checkers is enabled and is not in the trapping mode.

Comment should say why.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:983
+// Add offset.type metadata to the newly created offset global variable.
+OffsetGV->addOffsetTypeMetadata(TypeId, Offset);
+

I wouldn't add a function to GlobalObject for this, you can add the metadata 
here directly since this is the only place in the code where you need to add it.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1767
 
+  bool InBounds = shouldInterleaveVTables(VTableClass) ? false : true;
   return llvm::ConstantExpr::getGetElementPtr(VTable->getValueType(), VTable,

Remind me why this needs to be here? (And the explanation needs to be in a 
comment.)



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1150
 
+  Opts.EnableVTableInterleaving = Args.hasFlag(
+  OPT_enable_vtable_interleaving, OPT_disable_vtable_interleaving, false);

Use hasArg here instead, there's no need to support a flag for disabling 
interleaving in the frontend.


https://reviews.llvm.org/D51905



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

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Can you update this patch please? Then I will commit.


https://reviews.llvm.org/D50372



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

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341989: Introduce the VTable interleaving scheme to the CFI 
design documentation (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50372?vs=164964&id=164967#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50372

Files:
  docs/ControlFlowIntegrityDesign.rst

Index: docs/ControlFlowIntegrityDesign.rst
===
--- docs/ControlFlowIntegrityDesign.rst
+++ docs/ControlFlowIntegrityDesign.rst
@@ -274,6 +274,154 @@
 need to check that the address is in range and well aligned. This is more
 likely to occur if the virtual tables are padded.
 
+Forward-Edge CFI for Virtual Calls by Interleaving Virtual Tables
+-
+
+Dimitar et. al. proposed a novel approach that interleaves virtual tables in [1]_.  
+This approach is more efficient in terms of space because padding and bit vectors are no longer needed. 
+At the same time, it is also more efficient in terms of performance because in the interleaved layout 
+address points of the virtual tables are consecutive, thus the validity check of a virtual 
+vtable pointer is always a range check. 
+
+At a high level, the interleaving scheme consists of three steps: 1) split virtual table groups into 
+separate virtual tables, 2) order virtual tables by a pre-order traversal of the class hierarchy 
+and 3) interleave virtual tables.
+
+The interleaving scheme implemented in LLVM is inspired by [1]_ but has its own
+enhancements (more in `Interleave virtual tables`_).
+
+.. [1] `Protecting C++ Dynamic Dispatch Through VTable Interleaving `_. Dimitar Bounov, Rami Gökhan Kıcı, Sorin Lerner.
+
+Split virtual table groups into separate virtual tables
+~~~
+
+The Itanium C++ ABI glues multiple individual virtual tables for a class into a combined virtual table (virtual table group). 
+The interleaving scheme, however, can only work with individual virtual tables so it must split the combined virtual tables first.
+In comparison, the old scheme does not require the splitting but it is more efficient when the combined virtual tables have been split.
+The `GlobalSplit`_ pass is responsible for splitting combined virtual tables into individual ones. 
+
+.. _GlobalSplit: https://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalSplit.cpp?view=markup
+
+Order virtual tables by a pre-order traversal of the class hierarchy 
+
+
+This step is common to both the old scheme described above and the interleaving scheme. 
+For the interleaving scheme, since the combined virtual tables have been split in the previous step, 
+this step ensures that for any class all the compatible virtual tables will appear consecutively. 
+For the old scheme, the same property may not hold since it may work on combined virtual tables. 
+
+For example, consider the following four C++ classes:
+
+.. code-block:: c++
+
+  struct A {
+virtual void f1();
+  };
+
+  struct B : A {
+virtual void f1();
+virtual void f2();
+  };
+
+  struct C : A {
+virtual void f1();
+virtual void f3();
+  };
+
+  struct D : B {
+virtual void f1();
+virtual void f2();
+  };
+
+This step will arrange the virtual tables for A, B, C, and D in the order of *vtable-of-A, vtable-of-B, vtable-of-D, vtable-of-C*.
+
+Interleave virtual tables
+~
+
+This step is where the interleaving scheme deviates from the old scheme. Instead of laying out 
+whole virtual tables in the previously computed order, the interleaving scheme lays out table 
+entries of the virtual tables strategically to ensure the following properties:  
+
+(1) offset-to-top and RTTI fields layout property
+
+The Itanium C++ ABI specifies that offset-to-top and RTTI fields appear at the offsets behind the 
+address point. Note that libraries like libcxxabi do assume this property. 
+
+(2) virtual function entry layout property
+
+For each virtual function the distance between an virtual table entry for this function and the corresponding 
+address point is always the same. This property ensures that dynamic dispatch still works with the interleaving layout.
+
+Note that the interleaving scheme in the CFI implementation guarantees both properties above whereas the original scheme proposed   
+in [1]_ only guarantees the second property. 
+
+To illustrate how the interleaving algorithm works, let us continue with the running example.
+The algorithm first separates all the virtual table entries into two work lists. To do so, 
+it starts by allocating two work lists, one initialized with all the offset-to-top entries of virtual tables in the order 
+computed in the last step, one initialized 

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a subscriber: cfe-commits.
pcc added inline comments.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3215
   // Emit the standard library with external linkage.
   llvm::GlobalVariable::LinkageTypes Linkage;
   if (IsStdLib)

`IsStdLib` will always be false at this point because of the if statement on 
line 3211 so I think you can change this to just 
`llvm::GlobalVariable::LinkageTypes Linkage = getTypeInfoLinkage(CGM, Ty);` and 
fold the call to `IsStandardLibraryRTTIDescriptor(Ty)` into the if statement.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3239-3241
+} else if (RD && RD->hasAttr() &&
+   ShouldUseExternalRTTIDescriptor(CGM, Ty)) {
+  DLLStorageClass = llvm::GlobalValue::DLLImportStorageClass;

Similarly, `ShouldUseExternalRTTIDescriptor(CGM, Ty)` should always return 
false here because of the if statement, so this code is dead.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3408
 
   if (CGM.getTriple().isWindowsItaniumEnvironment()) {
+TypeName->setDLLStorageClass(DLLStorageClass);

You can remove the if statement, setting the storage class to default has no 
effect here because it is initialized to default.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3411-3417
+if (DLLStorageClass == llvm::GlobalValue::DLLImportStorageClass) {
   // Because the typename and the typeinfo are DLL import, convert them to
   // declarations rather than definitions.  The initializers still need to
   // be constructed to calculate the type for the declarations.
   TypeName->setInitializer(nullptr);
   GV->setInitializer(nullptr);
 }

This code is also dead.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3704
 
-void ItaniumCXXABI::EmitFundamentalRTTIDescriptors(bool DLLExport) {
+void ItaniumCXXABI::EmitFundamentalRTTIDescriptors(
+const TypeInfoOptions* Options) {

You might consider inlining `EmitFundamentalRTTIDescriptor` into this function 
and having it take a `CXXRecordDecl`. Then you wouldn't need to pass the 
`TypeInfoOptions` structure around, you can just compute the properties in this 
function outside of the loop.


https://reviews.llvm.org/D49109



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

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:211-224
+  std::string ErrorMsg1;
+  EngineBuilder builder1(std::move(Owner1));
+  builder1.setMArch(MArch);
+  builder1.setMCPU(getCPUStr());
+  builder1.setMAttrs(getFeatureList());
+  builder1.setErrorStr(&ErrorMsg1);
+  builder1.setEngineKind(EngineKind::JIT);

Can you move this code (and maybe more of the duplicated code below) into a 
function and call it twice?


Repository:
  rC Clang

https://reviews.llvm.org/D49526



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

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D49109



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

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

[Peter Collingbourne via Phabricator via cfe-commits]

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

Sorry, I noticed that this patch is missing a test case. Can you add one please?


https://reviews.llvm.org/D49109



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

[PATCH] D49704: Fix typo in test/CodeGen/Mips/dins.ll

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D49704



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

[PATCH] D49109: Borrow visibility from __fundamental_type_info for generated fundamental type infos

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D49109



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

[PATCH] D49745: Generate fundamental type info with weak linkage

[Peter Collingbourne via Phabricator via cfe-commits]

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

As I mentioned in https://bugs.llvm.org/show_bug.cgi?id=38281 I don't see a 
good reason to do this. Also extern_weak is not an appropriate linkage for 
definitions, only declarations.


https://reviews.llvm.org/D49745



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

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:147
+  builder.setUseOrcMCJITReplacement(false);
+  builder.setMCJITMemoryManager(make_unique());
+  builder.setOptLevel(OLvl);

morehouse wrote:
> This uses `llvm:make_unique`, which was written when LLVM used C++11.  But 
> since LLVM now uses C++14, I think we should use `std::make_unique` instead.
LLVM doesn't use C++14 yet.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



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

[PATCH] D42725: IRGen: Move vtable load after argument evaluation.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324286: IRGen: Move vtable load after argument evaluation. 
(authored by pcc, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42725?vs=132094&id=132895#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42725

Files:
  cfe/trunk/lib/CodeGen/CGCXXABI.h
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGCall.h
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
  cfe/trunk/test/CodeGenCXX/cfi-vcall-check-after-args.cpp
  cfe/trunk/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
  
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-this-adjustment.cpp

Index: cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
===
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
@@ -114,16 +114,15 @@
 // the caller site.
 //
 // CHECK: %[[CHILD_i8:.*]] = bitcast %struct.ChildOverride* %[[CHILD]] to i8*
+// CHECK: %[[RIGHT:.*]] = getelementptr inbounds i8, i8* %[[CHILD_i8]], i32 4
 //
+// CHECK: %[[CHILD_i8:.*]] = bitcast %struct.ChildOverride* %[[CHILD]] to i8*
 // CHECK: %[[VFPTR_i8:.*]] = getelementptr inbounds i8, i8* %[[CHILD_i8]], i32 4
 // CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VFPTR_i8]] to void (i8*)***
 // CHECK: %[[VFTABLE:.*]] = load void (i8*)**, void (i8*)*** %[[VFPTR]]
 // CHECK: %[[VFUN:.*]] = getelementptr inbounds void (i8*)*, void (i8*)** %[[VFTABLE]], i64 0
 // CHECK: %[[VFUN_VALUE:.*]] = load void (i8*)*, void (i8*)** %[[VFUN]]
 //
-// CHECK: %[[CHILD_i8:.*]] = bitcast %struct.ChildOverride* %[[CHILD]] to i8*
-// CHECK: %[[RIGHT:.*]] = getelementptr inbounds i8, i8* %[[CHILD_i8]], i32 4
-//
 // CHECK: call x86_thiscallcc void %[[VFUN_VALUE]](i8* %[[RIGHT]])
 // CHECK: ret
 }
Index: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-this-adjustment.cpp
===
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-this-adjustment.cpp
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-this-adjustment.cpp
@@ -156,23 +156,17 @@
 
 // BITCODE-LABEL: define {{.*}}\01?ffun@test4@@YAXAAUC@1@@Z
 void ffun(C &c) {
-  // BITCODE: load
-  // BITCODE: bitcast
-  // BITCODE: bitcast
   // BITCODE: [[THIS1:%.+]] = bitcast %"struct.test4::C"* {{.*}} to i8*
   // BITCODE: [[THIS2:%.+]] = getelementptr inbounds i8, i8* [[THIS1]], i32 4
-  // BITCODE-NEXT: call x86_thiscallcc {{.*}}(i8* [[THIS2]])
+  // BITCODE: call x86_thiscallcc {{.*}}(i8* [[THIS2]])
   c.bar();
 }
 
 // BITCODE-LABEL: define {{.*}}\01?fop@test4@@YAXAAUC@1@@Z
 void fop(C &c) {
-  // BITCODE: load
-  // BITCODE: bitcast
-  // BITCODE: bitcast
   // BITCODE: [[THIS1:%.+]] = bitcast %"struct.test4::C"* {{.*}} to i8*
   // BITCODE: [[THIS2:%.+]] = getelementptr inbounds i8, i8* [[THIS1]], i32 4
-  // BITCODE-NEXT: call x86_thiscallcc {{.*}}(i8* [[THIS2]])
+  // BITCODE: call x86_thiscallcc {{.*}}(i8* [[THIS2]])
   -c;
 }
 
Index: cfe/trunk/test/CodeGenCXX/cfi-vcall-check-after-args.cpp
===
--- cfe/trunk/test/CodeGenCXX/cfi-vcall-check-after-args.cpp
+++ cfe/trunk/test/CodeGenCXX/cfi-vcall-check-after-args.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck %s
+
+struct A {
+  virtual void f(int);
+};
+
+int g();
+void f(A *a) {
+  // CHECK: call i32 @_Z1gv()
+  // CHECK: call i1 @llvm.type.test
+  a->f(g());
+}
Index: cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
===
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -163,20 +163,20 @@
 // CHECK: %[[VBENTRY:.*]] = getelementptr inbounds i32, i32* %[[VBTABLE]], i32 1
 // CHECK: %[[VBOFFSET32:.*]] = load i32, i32* %[[VBENTRY]]
 // CHECK: %[[VBOFFSET:.*]] = add nsw i32 0, %[[VBOFFSET32]]
-// CHECK: %[[VBASE_i8:.*]] = getelementptr inbounds i8, i8* %[[OBJ_i8]], i32 %[[VBOFFSET]]
-// CHECK: %[[VFPTR:.*]] = bitcast i8* %[[VBASE_i8]] to void (i8*)***
-// CHECK: %[[VFTABLE:.*]] = load void (i8*)**, void (i8*)*** %[[VFPTR]]
-// CHECK: %[[VFUN:.*]] = getelementptr inbounds void (i8*)*, void (i8*)** %[[VFTABLE]], i64 2
-// CHECK: %[[VFUN_VALUE:.*]] = load void (i8*)*, void (i8*)** %[[VFUN]]
+// CHECK: %[[VBASE:.*]] 

[PATCH] D10831: Attach attribute "trap-func-name" to IR function

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.
Herald added subscribers: llvm-commits, mehdi_amini.

Maybe I'm missing something, but I don't see why we need this attribute. 
Couldn't clang have been changed to implement `-ftrap-function` by generating a 
call to the trap function instead of emitting an `llvm.trap` intrinsic call?


Repository:
  rL LLVM

https://reviews.llvm.org/D10831



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

[PATCH] D42680: [ThinLTO] Ignore object files with no ThinLTO modules if -fthinlto-index= is set

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/test/CodeGen/thinlto_backend.ll:27
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c 
-fthinlto-index=%t.thinlto.bc
+; RUN: llvm-nm %t4.o | wc -l | FileCheck %s --check-prefix=CHECK-SIZE-EMPTY
+; CHECK-SIZE-EMPTY: {{^0$}}

`llvm-nm %t4.o | count 0`


https://reviews.llvm.org/D42680



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

[PATCH] D42503: libcxx: Unbreak external thread library configuration.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX325723: libcxx: Unbreak external thread library 
configuration. (authored by pcc, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42503?vs=131344&id=135324#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D42503

Files:
  include/__threading_support


Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -31,23 +31,23 @@
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
+#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
+defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \
+defined(_LIBCPP_HAS_THREAD_API_WIN32)
+#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
+#else
+#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
+#endif
+
 #if defined(__FreeBSD__) && defined(__clang__) && 
__has_attribute(no_thread_safety_analysis)
 #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS 
__attribute__((no_thread_safety_analysis))
 #else
 #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
-defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL)
-
-#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
-
-#elif defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
-
-#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
-
+#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 // Mutex
 typedef pthread_mutex_t __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
@@ -75,9 +75,6 @@
 
 #define _LIBCPP_TLS_DESTRUCTOR_CC
 #else
-
-#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
-
 // Mutex
 typedef void* __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER 0


Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -31,23 +31,23 @@
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
+#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
+defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \
+defined(_LIBCPP_HAS_THREAD_API_WIN32)
+#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
+#else
+#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
+#endif
+
 #if defined(__FreeBSD__) && defined(__clang__) && __has_attribute(no_thread_safety_analysis)
 #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis))
 #else
 #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
-defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL)
-
-#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
-
-#elif defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
-
-#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
-
+#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 // Mutex
 typedef pthread_mutex_t __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
@@ -75,9 +75,6 @@
 
 #define _LIBCPP_TLS_DESTRUCTOR_CC
 #else
-
-#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS
-
 // Mutex
 typedef void* __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER 0
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43579: [libcxx] Do not include the C math.h header before __config

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: include/math.h:1499
+// has previously been included.
+#if defined(_LIBCPP_MSVCRT) && defined(_USE_MATH_DEFINES)
+#include_next 

Nit: you don't actually need the ` && defined(_USE_MATH_DEFINES)` part.


https://reviews.llvm.org/D43579



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

[PATCH] D33525: [ThinLTO] Migrate ThinLTOBitcodeWriter to the new PM.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/test/CodeGen/thin_link_bitcode.c:5
+// RUN: %clang_cc1 -o %t -flto=thin -fexperimental-new-pass-manager 
-fthin-link-bitcode=%t.newpm -triple x86_64-unknown-linux-gnu -emit-llvm-bc %s
+// RUN: llvm-bcanalyzer -dump %t.newpm | FileCheck %s --check-prefix=NEW_PM
 int main (void) {

`s/NEW_PM/NO_DEBUG/` and remove line 12.

I think you also want to test the contents of `%t` here.


https://reviews.llvm.org/D33525



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

[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Have you considered writing the regular LTO summaries unconditionally if 
`-flto` was specified? That was how I imagined that the interface would look.

Also, how were you planning to expose the reference graph to the linker? I 
gather from your message that you are teaching your linker to read the module 
summary index directly from bitcode files. I wonder whether it would be worth 
trying to avoid needing to read summaries multiple times. The approach that I 
had in mind was to somehow teach the linker to add regular object files to the 
combined summary index by creating a "global value summary" for each section, 
with a reference for each relocation. (This would be similar to how we add 
regular LTO inputs to the combined summary in https://reviews.llvm.org/D33922.) 
Then LTO would run as usual. Any regular object sections would then naturally 
participate in the summary-based dead stripping that LTO already does.


https://reviews.llvm.org/D34156



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

[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D34156#779661, @tobiasvk wrote:

> In https://reviews.llvm.org/D34156#779270, @pcc wrote:
>
> > Have you considered writing the regular LTO summaries unconditionally if 
> > `-flto` was specified? That was how I imagined that the interface would 
> > look.
>
>
> Absolutely, if people are OK with that. I would have enabled it by default in 
> our tree anyway. Let me know if you prefer this (and other people if you have 
> objections).


I'd be fine with it. We may want to make it conditional on the target (e.g. the 
summary wouldn't be much use on Darwin targets right now), but let's see what 
others think.

>> I wonder whether it would be worth trying to avoid needing to read summaries 
>> multiple times. The approach that I had in mind was to somehow teach the 
>> linker to add regular object files to the combined summary index by creating 
>> a "global value summary" for each section, with a reference for each 
>> relocation. (This would be similar to how we add regular LTO inputs to the 
>> combined summary in https://reviews.llvm.org/D33922.) Then LTO would run as 
>> usual. Any regular object sections would then naturally participate in the 
>> summary-based dead stripping that LTO already does.
> 
> It could be done but I'm skeptical about this from a cost/benefit 
> perspective. We'd only save one additional read of the summaries, which is 
> pretty cheap anyway. The GC logic in the linker is non-trivial and doesn't 
> map particularly well to the combined summary (e.g. we'd have to deal with 
> the liveness of groups of symbols rather than individual ones when symbols 
> share an input section).

That's a fair point, and now that I think about it it does seem better for GC 
to be entirely driven by the linker, as you have done. Longer term we may 
consider replacing summary-based dead stripping entirely with linker-driven 
dead stripping.


https://reviews.llvm.org/D34156



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

[PATCH] D34156: [LTO] Add -femit-summary-index flag to emit summary for regular LTO

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D34156#780714, @mehdi_amini wrote:

> In https://reviews.llvm.org/D34156#780570, @tobiasvk wrote:
>
> > - Change patch to always emit a module summary for regular LTO
> >
> >   I don't see any real downside of having a summary given the marginal time 
> > and space overhead it takes to construct and save it.
>
>
> I think some code/logic needs to be changed in LLVM first:
>
>   bool LTOModule::isThinLTO() {
> // Right now the detection is only based on the summary presence. We may 
> want
> // to add a dedicated flag at some point.
>


Thanks for pointing that out. Right now that function will ignore full LTO 
summaries because they are represented using a different record number, but 
once https://reviews.llvm.org/D33922 lands the `hasGlobalValueSummary` function 
will return true if the module contains a full LTO summary. I will update that 
function as part of https://reviews.llvm.org/D33922 to check for presence of a 
ThinLTO summary.


https://reviews.llvm.org/D34156



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

[PATCH] D34233: [CFI] Add ability to explicitly link classes

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Before looking at this patch in detail, can you please explain the need for 
this feature? How difficult would it be to change your code to avoid relying on 
invalid casts?


https://reviews.llvm.org/D34233



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

[PATCH] D34156: [LTO] Enable module summary emission by default for regular LTO

[Peter Collingbourne via Phabricator via cfe-commits]

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

Please confirm that we can still self host with full LTO now that 
https://reviews.llvm.org/D33922 has landed.

LGTM otherwise. Thanks!


https://reviews.llvm.org/D34156



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

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
Herald added subscribers: eraman, inglorion.

https://reviews.llvm.org/D34546

Files:
  clang/docs/ThinLTO.rst


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -126,6 +126,38 @@
 - lld (as of LLVM r296702):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
 
+Cache Pruning
+-
+
+To help keep the size of the cache under control, ThinLTO supports cache
+pruning. Cache pruning is supported with ld64 and ELF lld, but currently only
+ELF lld allows you to control the policy with a policy string.  The cache
+policy must be specified with a linker option.
+
+- ELF lld (as of LLVM r298036):
+  ``-Wl,--thinlto-cache-policy,POLICY``
+
+A policy string is a series of key-value pairs separated by ``:`` characters.
+Possible key-value pairs are:
+
+- ``cache_size=X%``: The maximum size for the cache directory is ``X`` percent
+  of the available space on the the disk. Set to 100 to indicate no limit,
+  50 to indicate that the cache size will not be left over half the available
+  disk space. A value over 100 is invalid. A value of 0 disables the percentage
+  size-based pruning. The default is 75%.
+
+- ``prune_after=Xs``, ``prune_after=Xm``, ``prune_after=Xh``: Sets the
+  expiration time for cache files to ``X`` seconds (or minutes, hours
+  respectively).  When a file hasn't been accessed for ``prune_after`` seconds,
+  it is removed from the cache. A value of 0 disables the expiration-based
+  pruning. The default is 1 week.
+
+- ``prune_interval=Xs``, ``prune_interval=Xm``, ``prune_interval=Xh``:
+  Sets the pruning interval to ``X`` seconds (or minutes, hours
+  respectively). This is intended to be used to avoid scanning the directory
+  too often. It does not impact the decision of which files to prune. A
+  value of 0 forces the scan to occur. The default is every 20 minutes.
+
 Clang Bootstrap
 ---
 


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -126,6 +126,38 @@
 - lld (as of LLVM r296702):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
 
+Cache Pruning
+-
+
+To help keep the size of the cache under control, ThinLTO supports cache
+pruning. Cache pruning is supported with ld64 and ELF lld, but currently only
+ELF lld allows you to control the policy with a policy string.  The cache
+policy must be specified with a linker option.
+
+- ELF lld (as of LLVM r298036):
+  ``-Wl,--thinlto-cache-policy,POLICY``
+
+A policy string is a series of key-value pairs separated by ``:`` characters.
+Possible key-value pairs are:
+
+- ``cache_size=X%``: The maximum size for the cache directory is ``X`` percent
+  of the available space on the the disk. Set to 100 to indicate no limit,
+  50 to indicate that the cache size will not be left over half the available
+  disk space. A value over 100 is invalid. A value of 0 disables the percentage
+  size-based pruning. The default is 75%.
+
+- ``prune_after=Xs``, ``prune_after=Xm``, ``prune_after=Xh``: Sets the
+  expiration time for cache files to ``X`` seconds (or minutes, hours
+  respectively).  When a file hasn't been accessed for ``prune_after`` seconds,
+  it is removed from the cache. A value of 0 disables the expiration-based
+  pruning. The default is 1 week.
+
+- ``prune_interval=Xs``, ``prune_interval=Xm``, ``prune_interval=Xh``:
+  Sets the pruning interval to ``X`` seconds (or minutes, hours
+  respectively). This is intended to be used to avoid scanning the directory
+  too often. It does not impact the decision of which files to prune. A
+  value of 0 forces the scan to occur. The default is every 20 minutes.
+
 Clang Bootstrap
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/docs/ThinLTO.rst:160
+  value of 0 forces the scan to occur. The default is every 20 minutes.
+
 Clang Bootstrap

mehdi_amini wrote:
> Why do we document linker-specific option in clang docs? Is it usual?
It does seem a little odd, but it's in the same place as the other 
linker-specific options above. I was also imagining that we would eventually 
make this into a clang driver option (similar to the cache location as you 
mentioned in D30522). At that point the clang docs would indeed be the right 
place to document it.


https://reviews.llvm.org/D34546



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

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306125: docs: Add documentation for the ThinLTO cache 
pruning policy string. (authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D34546?vs=103682&id=103743#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34546

Files:
  cfe/trunk/docs/ThinLTO.rst


Index: cfe/trunk/docs/ThinLTO.rst
===
--- cfe/trunk/docs/ThinLTO.rst
+++ cfe/trunk/docs/ThinLTO.rst
@@ -126,6 +126,38 @@
 - lld (as of LLVM r296702):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
 
+Cache Pruning
+-
+
+To help keep the size of the cache under control, ThinLTO supports cache
+pruning. Cache pruning is supported with ld64 and ELF lld, but currently only
+ELF lld allows you to control the policy with a policy string.  The cache
+policy must be specified with a linker option.
+
+- ELF lld (as of LLVM r298036):
+  ``-Wl,--thinlto-cache-policy,POLICY``
+
+A policy string is a series of key-value pairs separated by ``:`` characters.
+Possible key-value pairs are:
+
+- ``cache_size=X%``: The maximum size for the cache directory is ``X`` percent
+  of the available space on the the disk. Set to 100 to indicate no limit,
+  50 to indicate that the cache size will not be left over half the available
+  disk space. A value over 100 is invalid. A value of 0 disables the percentage
+  size-based pruning. The default is 75%.
+
+- ``prune_after=Xs``, ``prune_after=Xm``, ``prune_after=Xh``: Sets the
+  expiration time for cache files to ``X`` seconds (or minutes, hours
+  respectively).  When a file hasn't been accessed for ``prune_after`` seconds,
+  it is removed from the cache. A value of 0 disables the expiration-based
+  pruning. The default is 1 week.
+
+- ``prune_interval=Xs``, ``prune_interval=Xm``, ``prune_interval=Xh``:
+  Sets the pruning interval to ``X`` seconds (or minutes, hours
+  respectively). This is intended to be used to avoid scanning the directory
+  too often. It does not impact the decision of which files to prune. A
+  value of 0 forces the scan to occur. The default is every 20 minutes.
+
 Clang Bootstrap
 ---
 


Index: cfe/trunk/docs/ThinLTO.rst
===
--- cfe/trunk/docs/ThinLTO.rst
+++ cfe/trunk/docs/ThinLTO.rst
@@ -126,6 +126,38 @@
 - lld (as of LLVM r296702):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
 
+Cache Pruning
+-
+
+To help keep the size of the cache under control, ThinLTO supports cache
+pruning. Cache pruning is supported with ld64 and ELF lld, but currently only
+ELF lld allows you to control the policy with a policy string.  The cache
+policy must be specified with a linker option.
+
+- ELF lld (as of LLVM r298036):
+  ``-Wl,--thinlto-cache-policy,POLICY``
+
+A policy string is a series of key-value pairs separated by ``:`` characters.
+Possible key-value pairs are:
+
+- ``cache_size=X%``: The maximum size for the cache directory is ``X`` percent
+  of the available space on the the disk. Set to 100 to indicate no limit,
+  50 to indicate that the cache size will not be left over half the available
+  disk space. A value over 100 is invalid. A value of 0 disables the percentage
+  size-based pruning. The default is 75%.
+
+- ``prune_after=Xs``, ``prune_after=Xm``, ``prune_after=Xh``: Sets the
+  expiration time for cache files to ``X`` seconds (or minutes, hours
+  respectively).  When a file hasn't been accessed for ``prune_after`` seconds,
+  it is removed from the cache. A value of 0 disables the expiration-based
+  pruning. The default is 1 week.
+
+- ``prune_interval=Xs``, ``prune_interval=Xm``, ``prune_interval=Xh``:
+  Sets the pruning interval to ``X`` seconds (or minutes, hours
+  respectively). This is intended to be used to avoid scanning the directory
+  too often. It does not impact the decision of which files to prune. A
+  value of 0 forces the scan to occur. The default is every 20 minutes.
+
 Clang Bootstrap
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41319: libcxx: Fix for basic_stringbuf::seekoff() after r320604.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: EricWF, mclow.lists.

As a result of this change, the basic_stringbuf constructor that
takes a mode ends up leaving __hm_ set to 0, causing the comparison
"__hm_ - __str_.data() < __noff" in seekoff() to succeed, which caused
the function to incorrectly return -1. The fix is to account for the
possibility of __hm_ being 0 when computing the distance from __hm_
to the start of the string.


https://reviews.llvm.org/D41319

Files:
  libcxx/include/sstream
  
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp


Index: 
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
===
--- 
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
+++ 
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
@@ -20,6 +20,30 @@
 
 int main()
 {
+{
+std::stringbuf sb(std::ios_base::in);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::out) == 
-1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::in) == 0);
+}
+{
+std::stringbuf sb(std::ios_base::out);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::out) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::out) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::out) == 0);
+}
 {
 std::stringbuf sb("0123456789", std::ios_base::in);
 assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
Index: libcxx/include/sstream
===
--- libcxx/include/sstream
+++ libcxx/include/sstream
@@ -577,6 +577,7 @@
 if ((__wch & (ios_base::in | ios_base::out)) == (ios_base::in | 
ios_base::out)
 && __way == ios_base::cur)
 return pos_type(-1);
+ptrdiff_t __hm = __hm_ == nullptr ? 0 : __hm_ - __str_.data();
 off_type __noff;
 switch (__way)
 {
@@ -590,13 +591,13 @@
 __noff = this->pptr() - this->pbase();
 break;
 case ios_base::end:
-__noff = __hm_ - __str_.data();
+__noff = __hm;
 break;
 default:
 return pos_type(-1);
 }
 __noff += __off;
-if (__noff < 0 || __hm_ - __str_.data() < __noff)
+if (__noff < 0 || __hm < __noff)
 return pos_type(-1);
 if (__noff != 0)
 {


Index: libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
===
--- libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
+++ libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
@@ -20,6 +20,30 @@
 
 int main()
 {
+{
+std::stringbuf sb(std::ios_base::in);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::in) == 0);
+}
+{
+std::stringbuf sb(std::ios_base::out);
+   

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Would it be possible to fix this by stripping the noexcept specifiers from both 
the function type used in the check and the one that is embedded in the prefix 
data? The downside is that we won't catch the case where the caller has a 
noexcept specifier and the callee doesn't, but that seems like an edge case to 
me, and we can think about fixing it in other ways later.


Repository:
  rC Clang

https://reviews.llvm.org/D40720



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

[PATCH] D41319: libcxx: Fix for basic_stringbuf::seekoff() after r320604.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 127609.
pcc added a comment.

- Make __hm const


https://reviews.llvm.org/D41319

Files:
  libcxx/include/sstream
  
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp


Index: 
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
===
--- 
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
+++ 
libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
@@ -20,6 +20,30 @@
 
 int main()
 {
+{
+std::stringbuf sb(std::ios_base::in);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::out) == 
-1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::in) == 0);
+}
+{
+std::stringbuf sb(std::ios_base::out);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::out) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::out) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::out) == 0);
+}
 {
 std::stringbuf sb("0123456789", std::ios_base::in);
 assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
Index: libcxx/include/sstream
===
--- libcxx/include/sstream
+++ libcxx/include/sstream
@@ -577,6 +577,7 @@
 if ((__wch & (ios_base::in | ios_base::out)) == (ios_base::in | 
ios_base::out)
 && __way == ios_base::cur)
 return pos_type(-1);
+const ptrdiff_t __hm = __hm_ == nullptr ? 0 : __hm_ - __str_.data();
 off_type __noff;
 switch (__way)
 {
@@ -590,13 +591,13 @@
 __noff = this->pptr() - this->pbase();
 break;
 case ios_base::end:
-__noff = __hm_ - __str_.data();
+__noff = __hm;
 break;
 default:
 return pos_type(-1);
 }
 __noff += __off;
-if (__noff < 0 || __hm_ - __str_.data() < __noff)
+if (__noff < 0 || __hm < __noff)
 return pos_type(-1);
 if (__noff != 0)
 {


Index: libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
===
--- libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
+++ libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
@@ -20,6 +20,30 @@
 
 int main()
 {
+{
+std::stringbuf sb(std::ios_base::in);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::in) == 0);
+}
+{
+std::stringbuf sb(std::ios_base::out);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | std::ios_base::out) == -1);
+asse

[PATCH] D41319: libcxx: Fix for basic_stringbuf::seekoff() after r320604.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
pcc marked an inline comment as done.
Closed by commit rL321124: libcxx: Fix for basic_stringbuf::seekoff() after 
r320604. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41319?vs=127609&id=127611#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41319

Files:
  libcxx/trunk/include/sstream
  
libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp


Index: libcxx/trunk/include/sstream
===
--- libcxx/trunk/include/sstream
+++ libcxx/trunk/include/sstream
@@ -577,6 +577,7 @@
 if ((__wch & (ios_base::in | ios_base::out)) == (ios_base::in | 
ios_base::out)
 && __way == ios_base::cur)
 return pos_type(-1);
+const ptrdiff_t __hm = __hm_ == nullptr ? 0 : __hm_ - __str_.data();
 off_type __noff;
 switch (__way)
 {
@@ -590,13 +591,13 @@
 __noff = this->pptr() - this->pbase();
 break;
 case ios_base::end:
-__noff = __hm_ - __str_.data();
+__noff = __hm;
 break;
 default:
 return pos_type(-1);
 }
 __noff += __off;
-if (__noff < 0 || __hm_ - __str_.data() < __noff)
+if (__noff < 0 || __hm < __noff)
 return pos_type(-1);
 if (__noff != 0)
 {
Index: 
libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
===
--- 
libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
+++ 
libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
@@ -21,6 +21,30 @@
 int main()
 {
 {
+std::stringbuf sb(std::ios_base::in);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::out) == 
-1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::in) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::in) == 0);
+}
+{
+std::stringbuf sb(std::ios_base::out);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in) == -1);
+assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(-3, std::ios_base::end, std::ios_base::in | 
std::ios_base::out) == -1);
+assert(sb.pubseekoff(0, std::ios_base::beg, std::ios_base::out) == 0);
+assert(sb.pubseekoff(0, std::ios_base::cur, std::ios_base::out) == 0);
+assert(sb.pubseekoff(0, std::ios_base::end, std::ios_base::out) == 0);
+}
+{
 std::stringbuf sb("0123456789", std::ios_base::in);
 assert(sb.pubseekoff(3, std::ios_base::beg, std::ios_base::out) == -1);
 assert(sb.pubseekoff(3, std::ios_base::cur, std::ios_base::out) == -1);


Index: libcxx/trunk/include/sstream
===
--- libcxx/trunk/include/sstream
+++ libcxx/trunk/include/sstream
@@ -577,6 +577,7 @@
 if ((__wch & (ios_base::in | ios_base::out)) == (ios_base::in | ios_base::out)
 && __way == ios_base::cur)
 return pos_type(-1);
+const ptrdiff_t __hm = __hm_ == nullptr ? 0 : __hm_ - __str_.data();
 off_type __noff;
 switch (__way)
 {
@@ -590,13 +591,13 @@
 __noff = this->pptr() - this->pbase();
 break;
 case ios_base::end:
-__noff = __hm_ - __str_.data();
+__noff = __hm;
 break;
 default:
 return pos_type(-1);
 }
 __noff += __off;
-if (__noff < 0 || __hm_ - __str_.data() < __noff)
+if (__noff < 0 || __hm < __noff)
 return pos_type(-1);
 if (__noff != 0)
 {
Index: libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
===
--- libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
+++ libcxx/trunk/test/std/input.output/string.streams/stringbuf/stringbuf.virtuals/seekoff.pass.cpp
@@ -21,

[PATCH] D43805: Optionally use nameless IR types

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Why is this a driver flag? This seems like it ought to be a cc1-only flag to me.


Repository:
  rC Clang

https://reviews.llvm.org/D43805



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

[PATCH] D43805: Optionally use nameless IR types

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

> If the backend will be changed so that it will not depend on IR type names

Are you referring to https://reviews.llvm.org/D43199? If so it seems to me that 
this should be a cc1 flag that defaults to whether `-flto=thin` is passed. In 
any case it seems like a bad idea to deliberately generate different code 
depending on whether we were compiled with NDEBUG.


Repository:
  rC Clang

https://reviews.llvm.org/D43805



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

[PATCH] D47567: [wip] Implement CFI for indirect calls via a member function pointer.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: vlad.tsyrklevich.
Herald added a subscriber: mgrang.

Similarly to CFI on virtual and indirect calls, this implementation
tries to use program type information to make the checks as precise
as possible.  The basic way that it works is as follows, where `C`
is the name of the class being defined or the target of a call and
the function type is assumed to be `void()`.

For virtual calls:

- Attach type metadata to the addresses of function pointers in vtables (not 
the functions themselves) of type `void (B::*)()` for each `B` that is a 
recursive dynamic base class of `C`, including `C` itself. This type metadata 
has an annotation that the type is for virtual calls (to distinguish it from 
the non-virtual case).
- At the call site, check that the computed address of the function pointer in 
the vtable has type `void (C::*)()`.

For non-virtual calls:

- Attach type metadata to each non-virtual member function whose address can be 
taken with a member function pointer. The type of a function in class `C` of 
type `void()` is each of the types `void (B::*)()` where `B` is a most-base 
class of `C`. A most-base class of `C` is defined as a recursive base class of 
`C`, including `C` itself, that does not have any bases.
- At the call site, check that the function pointer has one of the types `void 
(B::*)()` where `B` is a most-base class of `C`.

TODO:

- Implement a fallback for the case where the class type is incomplete at the 
call site.
- Implement non-trapping and cross-DSO support.
- Mark this as unsupported with the Microsoft ABI for now.
- Write tests.


https://reviews.llvm.org/D47567

Files:
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/SanitizerArgs.cpp

Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -44,7 +44,8 @@
   TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
   Nullability | LocalBounds | CFI,
   TrappingDefault = CFI,
-  CFIClasses = CFIVCall | CFINVCall | CFIDerivedCast | CFIUnrelatedCast,
+  CFIClasses =
+  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
   CompatibleWithMinimalRuntime = TrappingSupported,
 };
 
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -621,10 +621,26 @@
 VTableOffset = Builder.CreateTrunc(VTableOffset, CGF.Int32Ty);
 VTableOffset = Builder.CreateZExt(VTableOffset, CGM.PtrDiffTy);
   }
+  // Compute the address of the virtual function pointer.
   VTable = Builder.CreateGEP(VTable, VTableOffset);
+  VTable = Builder.CreateBitCast(VTable, FTy->getPointerTo()->getPointerTo());
+
+  // Check the address of the function pointer if CFI on member function
+  // pointers is enabled.
+  if (CGF.SanOpts.has(SanitizerKind::CFIMFCall)) {
+llvm::Metadata *MD =
+CGM.CreateMetadataIdentifierForVirtualMemPtrType(QualType(MPT, 0));
+llvm::Value *TypeId = llvm::MetadataAsValue::get(CGF.getLLVMContext(), MD);
+
+llvm::Value *CastedVTable = Builder.CreateBitCast(VTable, CGF.Int8PtrTy);
+llvm::Value *TypeTest = Builder.CreateCall(
+CGM.getIntrinsic(llvm::Intrinsic::type_test), {CastedVTable, TypeId});
+
+CGF.EmitTrapCheck(TypeTest);
+FnVirtual = Builder.GetInsertBlock();
+  }
 
   // Load the virtual function to call.
-  VTable = Builder.CreateBitCast(VTable, FTy->getPointerTo()->getPointerTo());
   llvm::Value *VirtualFn =
 Builder.CreateAlignedLoad(VTable, CGF.getPointerAlign(),
   "memptr.virtualfn");
@@ -636,6 +652,27 @@
   llvm::Value *NonVirtualFn =
 Builder.CreateIntToPtr(FnAsInt, FTy->getPointerTo(), "memptr.nonvirtualfn");
 
+  // Check the function pointer if CFI on member function pointers is enabled.
+  if (CGF.SanOpts.has(SanitizerKind::CFIMFCall)) {
+llvm::Value *Bit = Builder.getFalse();
+llvm::Value *CastedNonVirtualFn =
+Builder.CreateBitCast(NonVirtualFn, CGF.Int8PtrTy);
+for (const CXXRecordDecl *Base :
+ CGM.getMostBaseClasses(MPT->getClass()->getAsCXXRecordDecl())) {
+  llvm::Metadata *MD =
+  CGM.CreateMetadataIdentifierForType(getContext().getMemberPointerType(
+  MPT->getPointeeType(),
+  getContext().getRecordType(Base).getTypePtr()));
+  llvm::Value *TypeId = llvm::MetadataAsValue::get(CGF.getLLVMContext(), MD);
+
+  llvm::Value *TypeTest = Builder.CreateCall(
+  CGM.getIntrinsic(llvm::Intrinsic::type_test), {CastedNonVirtualFn, TypeId});
+  Bit = Builder.CreateOr(Bit, TypeTest);
+}
+CGF.EmitTrapCheck(Bit);
+FnNonVirtual = Bu

[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: tejohnson, dblaikie.
Herald added subscribers: JDevlieghere, hiraditya, eraman, inglorion, 
mehdi_amini.

https://reviews.llvm.org/D47597

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/thinlto-split-dwarf.c
  llvm/include/llvm/LTO/Config.h
  llvm/lib/LTO/LTOBackend.cpp


Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -291,14 +291,19 @@
 return;
 
   std::unique_ptr DwoOut;
+  SmallString<1024> DwoFile(Conf.DwoPath);
   if (!Conf.DwoDir.empty()) {
 std::error_code EC;
 if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir))
   report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " +
  EC.message());
 
-SmallString<1024> DwoFile(Conf.DwoDir);
+DwoFile = Conf.DwoDir;
 sys::path::append(DwoFile, std::to_string(Task) + ".dwo");
+  }
+
+  if (!DwoFile.empty()) {
+std::error_code EC;
 TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str();
 DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None);
 if (EC)
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -76,6 +76,11 @@
   /// The directory to store .dwo files.
   std::string DwoDir;
 
+  /// The path to write a .dwo file to. This should generally only be used when
+  /// running an individual backend directly via thinBackend(), as otherwise
+  /// all .dwo files will be written to the same path.
+  std::string DwoPath;
+
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
Index: clang/test/CodeGen/thinlto-split-dwarf.c
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-split-dwarf.c
@@ -0,0 +1,21 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \
+// RUN:   -flto=thin -emit-llvm-bc \
+// RUN:   -o %t.o %s
+
+// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
+// RUN:   -o %t2.index \
+// RUN:   -r=%t.o,main,px
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
+// RUN:   -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o
+
+// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O %s
+// RUN: llvm-readobj -sections %t.native.dwo | FileCheck --check-prefix=DWO %s
+
+// O-NOT: .dwo
+// DWO: .dwo
+
+int main() {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1161,6 +1161,7 @@
   Conf.DebugPassManager = CGOpts.DebugPassManager;
   Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
   Conf.RemarksFilename = CGOpts.OptRecordFile;
+  Conf.DwoPath = CGOpts.SplitDwarfFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) {


Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -291,14 +291,19 @@
 return;
 
   std::unique_ptr DwoOut;
+  SmallString<1024> DwoFile(Conf.DwoPath);
   if (!Conf.DwoDir.empty()) {
 std::error_code EC;
 if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir))
   report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " +
  EC.message());
 
-SmallString<1024> DwoFile(Conf.DwoDir);
+DwoFile = Conf.DwoDir;
 sys::path::append(DwoFile, std::to_string(Task) + ".dwo");
+  }
+
+  if (!DwoFile.empty()) {
+std::error_code EC;
 TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str();
 DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None);
 if (EC)
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -76,6 +76,11 @@
   /// The directory to store .dwo files.
   std::string DwoDir;
 
+  /// The path to write a .dwo file to. This should generally only be used when
+  /// running an individual backend directly via thinBackend(), as otherwise
+  /// all .dwo files will be written to the same path.
+  std::string DwoPath;
+
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
Index: clang/test/CodeGen/thinlto-split-dwarf.c
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-split-dwarf.c
@@ -0,0 +1,21 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \
+// RUN:   -flto=thin -emit-llvm-bc \
+// RUN:   -o %t.o %s
+
+// RUN: ll

[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333677: IRGen: Write .dwo files when -split-dwarf-file is 
used together with -fthinlto… (authored by pcc, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47597?vs=149323&id=149326#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47597

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/test/CodeGen/thinlto-split-dwarf.c
  llvm/trunk/include/llvm/LTO/Config.h
  llvm/trunk/lib/LTO/LTOBackend.cpp


Index: llvm/trunk/lib/LTO/LTOBackend.cpp
===
--- llvm/trunk/lib/LTO/LTOBackend.cpp
+++ llvm/trunk/lib/LTO/LTOBackend.cpp
@@ -291,14 +291,19 @@
 return;
 
   std::unique_ptr DwoOut;
+  SmallString<1024> DwoFile(Conf.DwoPath);
   if (!Conf.DwoDir.empty()) {
 std::error_code EC;
 if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir))
   report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " +
  EC.message());
 
-SmallString<1024> DwoFile(Conf.DwoDir);
+DwoFile = Conf.DwoDir;
 sys::path::append(DwoFile, std::to_string(Task) + ".dwo");
+  }
+
+  if (!DwoFile.empty()) {
+std::error_code EC;
 TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str();
 DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None);
 if (EC)
Index: llvm/trunk/include/llvm/LTO/Config.h
===
--- llvm/trunk/include/llvm/LTO/Config.h
+++ llvm/trunk/include/llvm/LTO/Config.h
@@ -76,6 +76,11 @@
   /// The directory to store .dwo files.
   std::string DwoDir;
 
+  /// The path to write a .dwo file to. This should generally only be used when
+  /// running an individual backend directly via thinBackend(), as otherwise
+  /// all .dwo files will be written to the same path.
+  std::string DwoPath;
+
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -1161,6 +1161,7 @@
   Conf.DebugPassManager = CGOpts.DebugPassManager;
   Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
   Conf.RemarksFilename = CGOpts.OptRecordFile;
+  Conf.DwoPath = CGOpts.SplitDwarfFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) {
Index: cfe/trunk/test/CodeGen/thinlto-split-dwarf.c
===
--- cfe/trunk/test/CodeGen/thinlto-split-dwarf.c
+++ cfe/trunk/test/CodeGen/thinlto-split-dwarf.c
@@ -0,0 +1,21 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux-gnu \
+// RUN:   -flto=thin -emit-llvm-bc \
+// RUN:   -o %t.o %s
+
+// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
+// RUN:   -o %t2.index \
+// RUN:   -r=%t.o,main,px
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
+// RUN:   -o %t.native.o -split-dwarf-file %t.native.dwo -x ir %t.o
+
+// RUN: llvm-readobj -sections %t.native.o | FileCheck --check-prefix=O %s
+// RUN: llvm-readobj -sections %t.native.dwo | FileCheck --check-prefix=DWO %s
+
+// O-NOT: .dwo
+// DWO: .dwo
+
+int main() {}


Index: llvm/trunk/lib/LTO/LTOBackend.cpp
===
--- llvm/trunk/lib/LTO/LTOBackend.cpp
+++ llvm/trunk/lib/LTO/LTOBackend.cpp
@@ -291,14 +291,19 @@
 return;
 
   std::unique_ptr DwoOut;
+  SmallString<1024> DwoFile(Conf.DwoPath);
   if (!Conf.DwoDir.empty()) {
 std::error_code EC;
 if (auto EC = llvm::sys::fs::create_directories(Conf.DwoDir))
   report_fatal_error("Failed to create directory " + Conf.DwoDir + ": " +
  EC.message());
 
-SmallString<1024> DwoFile(Conf.DwoDir);
+DwoFile = Conf.DwoDir;
 sys::path::append(DwoFile, std::to_string(Task) + ".dwo");
+  }
+
+  if (!DwoFile.empty()) {
+std::error_code EC;
 TM->Options.MCOptions.SplitDwarfFile = DwoFile.str().str();
 DwoOut = llvm::make_unique(DwoFile, EC, sys::fs::F_None);
 if (EC)
Index: llvm/trunk/include/llvm/LTO/Config.h
===
--- llvm/trunk/include/llvm/LTO/Config.h
+++ llvm/trunk/include/llvm/LTO/Config.h
@@ -76,6 +76,11 @@
   /// The directory to store .dwo files.
   std::string DwoDir;
 
+  /// The path to write a .dwo file to. This should generally only be used when
+  /// running an individual backend directly via thinBackend(), as otherwise
+  /// all .dwo files will be written to the same path.
+  std::string DwoPath;
+
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
Index: cfe/trunk/lib/Code

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46700



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

[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:435
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("-plugin-opt=objcopy=") + Objcopy));
+  CmdArgs.push_back(

You no longer need to pass `objcopy=`, see D47091.


https://reviews.llvm.org/D44788



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

[PATCH] D47567: [wip] Implement CFI for indirect calls via a member function pointer.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 151090.
pcc added a comment.
Herald added subscribers: steven_wu, kubamracek, mehdi_amini.

- Address TODOs


https://reviews.llvm.org/D47567

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/LTOVisibility.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/CodeGenCXX/cfi-mfcall-incomplete.cpp
  clang/test/CodeGenCXX/cfi-mfcall.cpp
  clang/test/CodeGenCXX/type-metadata.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_handlers.cc
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/test/cfi/mfcall.cpp

Index: compiler-rt/test/cfi/mfcall.cpp
===
--- /dev/null
+++ compiler-rt/test/cfi/mfcall.cpp
@@ -0,0 +1,94 @@
+// RUN: %clangxx_cfi -o %t %s
+// RUN: %expect_crash %run %t a
+// RUN: %expect_crash %run %t b
+// RUN: %expect_crash %run %t c
+// RUN: %expect_crash %run %t d
+// RUN: %expect_crash %run %t e
+// RUN: %run %t f
+// RUN: %run %t g
+
+// RUN: %clangxx_cfi_diag -o %t2 %s
+// RUN: %run %t2 a 2>&1 | FileCheck --check-prefix=A %s
+// RUN: %run %t2 b 2>&1 | FileCheck --check-prefix=B %s
+// RUN: %run %t2 c 2>&1 | FileCheck --check-prefix=C %s
+// RUN: %run %t2 d 2>&1 | FileCheck --check-prefix=D %s
+// RUN: %run %t2 e 2>&1 | FileCheck --check-prefix=E %s
+
+#include 
+#include 
+
+struct SBase1 {
+  void b1() {}
+};
+
+struct SBase2 {
+  void b2() {}
+};
+
+struct S : SBase1, SBase2 {
+  void f1() {}
+  int f2() { return 1; }
+  virtual void g1() {}
+  virtual int g2() { return 1; }
+  virtual int g3() { return 1; }
+};
+
+struct T {
+  void f1() {}
+  int f2() { return 2; }
+  virtual void g1() {}
+  virtual int g2() { return 2; }
+  virtual void g3() {}
+};
+
+typedef void (S::*S_void)();
+
+typedef int (S::*S_int)();
+typedef int (T::*T_int)();
+
+template 
+To bitcast(From f) {
+  assert(sizeof(To) == sizeof(From));
+  To t;
+  memcpy(&t, &f, sizeof(f));
+  return t;
+}
+
+int main(int argc, char **argv) {
+  S s;
+  T t;
+
+  switch (argv[1][0]) {
+case 'a':
+  // A: runtime error: control flow integrity check for type 'int (S::*)()' failed during non-virtual member function call
+  // A: note: S::f1() defined here
+  (s.*bitcast(&S::f1))();
+  break;
+case 'b':
+  // B: runtime error: control flow integrity check for type 'int (T::*)()' failed during non-virtual member function call
+  // B: note: S::f2() defined here
+  (t.*bitcast(&S::f2))();
+  break;
+case 'c':
+  // C: runtime error: control flow integrity check for type 'int (S::*)()' failed during virtual member function call
+  // C: note: vtable is of type 'S'
+  (s.*bitcast(&S::g1))();
+  break;
+case 'd':
+  // D: runtime error: control flow integrity check for type 'int (S::*)()' failed during virtual member function call
+  // D: note: vtable is of type 'T'
+  (reinterpret_cast(t).*&S::g2)();
+  break;
+case 'e':
+  // E: runtime error: control flow integrity check for type 'void (S::*)()' failed during virtual member function call
+  // E: note: vtable is of type 'S'
+  (s.*bitcast(&T::g3))();
+  break;
+case 'f':
+  (s.*&SBase1::b1)();
+  break;
+case 'g':
+  (s.*&SBase2::b2)();
+  break;
+  }
+}
Index: compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
===
--- compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
+++ compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
@@ -122,7 +122,11 @@
   case CFITCK_UnrelatedCast:
 CheckKindStr = "cast to unrelated type";
 break;
+  case CFITCK_VMFCall:
+CheckKindStr = "virtual member function call";
+break;
   case CFITCK_ICall:
+  case CFITCK_NVMFCall:
 Die();
   }
 
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -181,6 +181,8 @@
   CFITCK_DerivedCast,
   CFITCK_UnrelatedCast,
   CFITCK_ICall,
+  CFITCK_NVMFCall,
+  CFITCK_VMFCall,
 };
 
 struct CFICheckFailData {
Index: compiler-rt/lib/ubsan/ubsan_handlers.cc
===
--- compiler-rt/lib/ubsan/ubsan_handlers.cc
+++ compiler-rt/lib/ubsan/ubsan_handlers.cc
@@ -630,7 +630,7 @@
 
 static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function,
   ReportOptions Opts) {
-  if (Data->CheckKind != CFITCK_ICall)
+  if (Data->CheckKind != CFITCK_ICall && Data->CheckKind != CFITCK_NVMFCall)
 Die();
 
   SourceLocation Loc = Data->Loc.acquire();
@@ -641,9 +6

[PATCH] D47567: Implement CFI for indirect calls via a member function pointer.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 151272.
pcc added a comment.

- Add some more documentation to ControlFlowIntegrity.rst


https://reviews.llvm.org/D47567

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/LTOVisibility.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/CodeGenCXX/cfi-mfcall-incomplete.cpp
  clang/test/CodeGenCXX/cfi-mfcall.cpp
  clang/test/CodeGenCXX/type-metadata.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_handlers.cc
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/test/cfi/mfcall.cpp

Index: compiler-rt/test/cfi/mfcall.cpp
===
--- /dev/null
+++ compiler-rt/test/cfi/mfcall.cpp
@@ -0,0 +1,94 @@
+// RUN: %clangxx_cfi -o %t %s
+// RUN: %expect_crash %run %t a
+// RUN: %expect_crash %run %t b
+// RUN: %expect_crash %run %t c
+// RUN: %expect_crash %run %t d
+// RUN: %expect_crash %run %t e
+// RUN: %run %t f
+// RUN: %run %t g
+
+// RUN: %clangxx_cfi_diag -o %t2 %s
+// RUN: %run %t2 a 2>&1 | FileCheck --check-prefix=A %s
+// RUN: %run %t2 b 2>&1 | FileCheck --check-prefix=B %s
+// RUN: %run %t2 c 2>&1 | FileCheck --check-prefix=C %s
+// RUN: %run %t2 d 2>&1 | FileCheck --check-prefix=D %s
+// RUN: %run %t2 e 2>&1 | FileCheck --check-prefix=E %s
+
+#include 
+#include 
+
+struct SBase1 {
+  void b1() {}
+};
+
+struct SBase2 {
+  void b2() {}
+};
+
+struct S : SBase1, SBase2 {
+  void f1() {}
+  int f2() { return 1; }
+  virtual void g1() {}
+  virtual int g2() { return 1; }
+  virtual int g3() { return 1; }
+};
+
+struct T {
+  void f1() {}
+  int f2() { return 2; }
+  virtual void g1() {}
+  virtual int g2() { return 2; }
+  virtual void g3() {}
+};
+
+typedef void (S::*S_void)();
+
+typedef int (S::*S_int)();
+typedef int (T::*T_int)();
+
+template 
+To bitcast(From f) {
+  assert(sizeof(To) == sizeof(From));
+  To t;
+  memcpy(&t, &f, sizeof(f));
+  return t;
+}
+
+int main(int argc, char **argv) {
+  S s;
+  T t;
+
+  switch (argv[1][0]) {
+case 'a':
+  // A: runtime error: control flow integrity check for type 'int (S::*)()' failed during non-virtual member function call
+  // A: note: S::f1() defined here
+  (s.*bitcast(&S::f1))();
+  break;
+case 'b':
+  // B: runtime error: control flow integrity check for type 'int (T::*)()' failed during non-virtual member function call
+  // B: note: S::f2() defined here
+  (t.*bitcast(&S::f2))();
+  break;
+case 'c':
+  // C: runtime error: control flow integrity check for type 'int (S::*)()' failed during virtual member function call
+  // C: note: vtable is of type 'S'
+  (s.*bitcast(&S::g1))();
+  break;
+case 'd':
+  // D: runtime error: control flow integrity check for type 'int (S::*)()' failed during virtual member function call
+  // D: note: vtable is of type 'T'
+  (reinterpret_cast(t).*&S::g2)();
+  break;
+case 'e':
+  // E: runtime error: control flow integrity check for type 'void (S::*)()' failed during virtual member function call
+  // E: note: vtable is of type 'S'
+  (s.*bitcast(&T::g3))();
+  break;
+case 'f':
+  (s.*&SBase1::b1)();
+  break;
+case 'g':
+  (s.*&SBase2::b2)();
+  break;
+  }
+}
Index: compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
===
--- compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
+++ compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
@@ -122,7 +122,11 @@
   case CFITCK_UnrelatedCast:
 CheckKindStr = "cast to unrelated type";
 break;
+  case CFITCK_VMFCall:
+CheckKindStr = "virtual member function call";
+break;
   case CFITCK_ICall:
+  case CFITCK_NVMFCall:
 Die();
   }
 
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -181,6 +181,8 @@
   CFITCK_DerivedCast,
   CFITCK_UnrelatedCast,
   CFITCK_ICall,
+  CFITCK_NVMFCall,
+  CFITCK_VMFCall,
 };
 
 struct CFICheckFailData {
Index: compiler-rt/lib/ubsan/ubsan_handlers.cc
===
--- compiler-rt/lib/ubsan/ubsan_handlers.cc
+++ compiler-rt/lib/ubsan/ubsan_handlers.cc
@@ -630,7 +630,7 @@
 
 static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function,
   ReportOptions Opts) {
-  if (Data->CheckKind != CFITCK_ICall)
+  if (Data->CheckKind != CFITCK_ICall && Data->CheckKind != CFITCK_NVMFCall)
 Die();
 
   SourceLocation Loc = Data->Loc.acquire();
@@ -641,9 +641,12 @@
 
   Scoped

[PATCH] D48155: Teach Clang to emit address-significance tables.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: echristo, Bigcheese, rsmith.

By default, we emit an address-significance table on all ELF
targets when the integrated assembler is enabled. The emission of an
address-significance table can be controlled with the -faddrsig and
-fno-addrsig flags.

Depends on https://reviews.llvm.org/D48143


https://reviews.llvm.org/D48155

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/addrsig.c
  clang/test/Driver/addrsig.c

Index: clang/test/Driver/addrsig.c
===
--- /dev/null
+++ clang/test/Driver/addrsig.c
@@ -0,0 +1,6 @@
+// RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -gz -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
+// RUN: %clang -### -target x86_64-apple-darwin -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
+
+// ADDRSIG: -faddrsig
+// NO-ADDRSIG-NOT: -faddrsig
Index: clang/test/CodeGen/addrsig.c
===
--- /dev/null
+++ clang/test/CodeGen/addrsig.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -S %s -faddrsig -O -o - | FileCheck --check-prefix=ADDRSIG %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -S %s -O -o - | FileCheck --check-prefix=NO-ADDRSIG %s
+
+// ADDRSIG: .addrsig
+// ADDRSIG: .addrsig_sym g1
+// ADDRSIG-NOT: .addrsig_sym g2
+
+// NO-ADDRSIG-NOT: .addrsig
+
+extern const int g1[], g2[];
+
+const int *f1() {
+  return g1;
+}
+
+int f2() {
+  return g2[0];
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1119,6 +1119,8 @@
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
+  Opts.Addrsig = Args.hasArg(OPT_faddrsig);
+
   return Success;
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4798,6 +4798,11 @@
options::OPT_fno_complete_member_pointers, false))
 CmdArgs.push_back("-fcomplete-member-pointers");
 
+  if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
+   getToolChain().getTriple().isOSBinFormatELF() &&
+   getToolChain().useIntegratedAs()))
+CmdArgs.push_back("-faddrsig");
+
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
   Output.getType() == types::TY_Object &&
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -454,6 +454,7 @@
   Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
+  Options.EmitAddrsig = CodeGenOpts.Addrsig;
 
   if (CodeGenOpts.EnableSplitDwarf)
 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
Index: clang/include/clang/Frontend/CodeGenOptions.def
===
--- clang/include/clang/Frontend/CodeGenOptions.def
+++ clang/include/clang/Frontend/CodeGenOptions.def
@@ -335,6 +335,9 @@
 /// Whether to emit all vtables
 CODEGENOPT(ForceEmitVTables, 1, 0)
 
+/// Whether to emit an address-significance table into the object file.
+CODEGENOPT(Addrsig, 1, 0)
+
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -758,6 +758,10 @@
 def fno_profile_use : Flag<["-"], "fno-profile-use">,
 Alias;
 
+def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, CC1Option]>,
+  HelpText<"Emit an address-significance table">;
+def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, Flags<[CoreOption]>,
+  HelpText<"Don't emit an address-significance table">;
 def fblocks : Flag<["-"], "fblocks">, Group, Flags<[CC1Option]>,
   HelpText<"Enable the 'blocks' language feature">;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48155: Teach Clang to emit address-significance tables.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 151282.
pcc added a comment.

- Add some additional driver tests


https://reviews.llvm.org/D48155

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/addrsig.c
  clang/test/Driver/addrsig.c

Index: clang/test/Driver/addrsig.c
===
--- /dev/null
+++ clang/test/Driver/addrsig.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### -target x86_64-unknown-linux -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
+// RUN: %clang -### -target x86_64-unknown-linux -fno-integrated-as -faddrsig -c %s 2>&1 | FileCheck -check-prefix=ADDRSIG %s
+// RUN: %clang -### -target x86_64-unknown-linux -fno-addrsig -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
+// RUN: %clang -### -target x86_64-apple-darwin -c %s 2>&1 | FileCheck -check-prefix=NO-ADDRSIG %s
+
+// ADDRSIG: -faddrsig
+// NO-ADDRSIG-NOT: -faddrsig
Index: clang/test/CodeGen/addrsig.c
===
--- /dev/null
+++ clang/test/CodeGen/addrsig.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -S %s -faddrsig -O -o - | FileCheck --check-prefix=ADDRSIG %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -S %s -O -o - | FileCheck --check-prefix=NO-ADDRSIG %s
+
+// ADDRSIG: .addrsig
+// ADDRSIG: .addrsig_sym g1
+// ADDRSIG-NOT: .addrsig_sym g2
+
+// NO-ADDRSIG-NOT: .addrsig
+
+extern const int g1[], g2[];
+
+const int *f1() {
+  return g1;
+}
+
+int f2() {
+  return g2[0];
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1119,6 +1119,8 @@
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
+  Opts.Addrsig = Args.hasArg(OPT_faddrsig);
+
   return Success;
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4798,6 +4798,11 @@
options::OPT_fno_complete_member_pointers, false))
 CmdArgs.push_back("-fcomplete-member-pointers");
 
+  if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
+   getToolChain().getTriple().isOSBinFormatELF() &&
+   getToolChain().useIntegratedAs()))
+CmdArgs.push_back("-faddrsig");
+
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
   Output.getType() == types::TY_Object &&
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -454,6 +454,7 @@
   Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
+  Options.EmitAddrsig = CodeGenOpts.Addrsig;
 
   if (CodeGenOpts.EnableSplitDwarf)
 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
Index: clang/include/clang/Frontend/CodeGenOptions.def
===
--- clang/include/clang/Frontend/CodeGenOptions.def
+++ clang/include/clang/Frontend/CodeGenOptions.def
@@ -335,6 +335,9 @@
 /// Whether to emit all vtables
 CODEGENOPT(ForceEmitVTables, 1, 0)
 
+/// Whether to emit an address-significance table into the object file.
+CODEGENOPT(Addrsig, 1, 0)
+
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -758,6 +758,10 @@
 def fno_profile_use : Flag<["-"], "fno-profile-use">,
 Alias;
 
+def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, CC1Option]>,
+  HelpText<"Emit an address-significance table">;
+def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, Flags<[CoreOption]>,
+  HelpText<"Don't emit an address-significance table">;
 def fblocks : Flag<["-"], "fblocks">, Group, Flags<[CC1Option]>,
   HelpText<"Enable the 'blocks' language feature">;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48206: IRgen: Mark aliases of ctors and dtors as unnamed_addr.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: rsmith.

This is not only semantically correct but ensures that they will not
be marked as address-significant once https://reviews.llvm.org/D48155 lands.


https://reviews.llvm.org/D48206

Files:
  clang/lib/CodeGen/CGCXX.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/constructor-alias.cpp
  clang/test/CodeGenCXX/ctor-dtor-alias.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/dllexport-alias.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
  clang/test/CodeGenCXX/virtual-bases.cpp
  clang/test/CodeGenCXX/virtual-destructor-calls.cpp

Index: clang/test/CodeGenCXX/virtual-destructor-calls.cpp
===
--- clang/test/CodeGenCXX/virtual-destructor-calls.cpp
+++ clang/test/CodeGenCXX/virtual-destructor-calls.cpp
@@ -14,11 +14,11 @@
 };
 
 // Complete dtor: just an alias because there are no virtual bases.
-// CHECK: @_ZN1BD1Ev = alias {{.*}} @_ZN1BD2Ev
+// CHECK: @_ZN1BD1Ev = unnamed_addr alias {{.*}} @_ZN1BD2Ev
 
 // (aliases from C)
-// CHECK: @_ZN1CD2Ev = alias {{.*}}, bitcast {{.*}} @_ZN1BD2Ev
-// CHECK: @_ZN1CD1Ev = alias {{.*}} @_ZN1CD2Ev
+// CHECK: @_ZN1CD2Ev = unnamed_addr alias {{.*}}, bitcast {{.*}} @_ZN1BD2Ev
+// CHECK: @_ZN1CD1Ev = unnamed_addr alias {{.*}} @_ZN1CD2Ev
 
 // Base dtor: actually calls A's base dtor.
 // CHECK-LABEL: define void @_ZN1BD2Ev(%struct.B* %this) unnamed_addr
Index: clang/test/CodeGenCXX/virtual-bases.cpp
===
--- clang/test/CodeGenCXX/virtual-bases.cpp
+++ clang/test/CodeGenCXX/virtual-bases.cpp
@@ -4,7 +4,7 @@
   A();
 };
 
-// CHECK: @_ZN1AC1Ev = alias {{.*}} @_ZN1AC2Ev
+// CHECK: @_ZN1AC1Ev = unnamed_addr alias {{.*}} @_ZN1AC2Ev
 // CHECK-LABEL: define void @_ZN1AC2Ev(%struct.A* %this) unnamed_addr
 A::A() { }
 
Index: clang/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
===
--- clang/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
@@ -22,7 +22,7 @@
 void foo() {
   B b;
 }
-// CHECK-DAG: @"??1B@test2@@UAE@XZ" = dso_local alias void (%"struct.test2::B"*), bitcast (void (%"struct.test2::A"*)* @"??1A@test2@@UAE@XZ" to void (%"struct.test2::B"*)*)
+// CHECK-DAG: @"??1B@test2@@UAE@XZ" = dso_local unnamed_addr alias void (%"struct.test2::B"*), bitcast (void (%"struct.test2::A"*)* @"??1A@test2@@UAE@XZ" to void (%"struct.test2::B"*)*)
 }
 
 namespace test3 {
Index: clang/test/CodeGenCXX/dllexport.cpp
===
--- clang/test/CodeGenCXX/dllexport.cpp
+++ clang/test/CodeGenCXX/dllexport.cpp
@@ -641,7 +641,7 @@
   A::~A() { }
   B::~B() { }
   // Emit a alias definition of B's constructor.
-  // M32-DAG: @"??1B@UseDtorAlias@@QAE@XZ" = dso_local dllexport alias {{.*}} @"??1A@UseDtorAlias@@QAE@XZ"
+  // M32-DAG: @"??1B@UseDtorAlias@@QAE@XZ" = dso_local dllexport unnamed_addr alias {{.*}} @"??1A@UseDtorAlias@@QAE@XZ"
 }
 
 struct __declspec(dllexport) DefaultedCtorsDtors {
Index: clang/test/CodeGenCXX/dllexport-alias.cpp
===
--- clang/test/CodeGenCXX/dllexport-alias.cpp
+++ clang/test/CodeGenCXX/dllexport-alias.cpp
@@ -14,5 +14,5 @@
 
 A::~A() {}
 
-// CHECK: @_ZN1AC1Ev = dso_local dllexport alias void (%class.A*), void (%class.A*)* @_ZN1AC2Ev
-// CHECK: @_ZN1AD1Ev = dso_local dllexport alias void (%class.A*), void (%class.A*)* @_ZN1AD2Ev
+// CHECK: @_ZN1AC1Ev = dso_local dllexport unnamed_addr alias void (%class.A*), void (%class.A*)* @_ZN1AC2Ev
+// CHECK: @_ZN1AD1Ev = dso_local dllexport unnamed_addr alias void (%class.A*), void (%class.A*)* @_ZN1AD2Ev
Index: clang/test/CodeGenCXX/destructors.cpp
===
--- clang/test/CodeGenCXX/destructors.cpp
+++ clang/test/CodeGenCXX/destructors.cpp
@@ -96,15 +96,15 @@
 
 // complete destructor alias tested above
 
-// CHECK2-LABEL: @_ZN5test01AD1Ev = alias {{.*}} @_ZN5test01AD2Ev
+// CHECK2-LABEL: @_ZN5test01AD1Ev = unnamed_addr alias {{.*}} @_ZN5test01AD2Ev
 // CHECK2-LABEL: define void @_ZN5test01AD2Ev(%"struct.test0::A"* %this) unnamed_addr
 // CHECK2: invoke void @_ZN5test06MemberD1Ev
 // CHECK2:   unwind label [[MEM_UNWIND:%[a-zA-Z0-9.]+]]
 // CHECK2: invoke void @_ZN5test04BaseD2Ev
 // CHECK2:   unwind label [[BASE_UNWIND:%[a-zA-Z0-9.]+]]
 
 // In C++11, the destructors are often known not to throw.
-// CHECK2v11-LABEL: @_ZN5test01AD1Ev = alias {{.*}} @_ZN5test01AD2Ev
+// CHECK2v11-LABEL: @_ZN5test01AD1Ev = unnamed_addr alias {{.*}} @_ZN5test01AD2Ev
 // CHECK2v11-LABEL: define void @_ZN5test01AD2Ev(%"struct.test0::A"* %this) unnamed_addr
 // CHECK2v11: call void @_ZN5test06MemberD1Ev
 // CHECK2v11: call void @_ZN5test04BaseD2Ev
@@ -153,15 +

[PATCH] D48206: IRgen: Mark aliases of ctors and dtors as unnamed_addr.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC334982: IRgen: Mark aliases of ctors and dtors as 
unnamed_addr. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48206?vs=151455&id=151787#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48206

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/constructor-alias.cpp
  test/CodeGenCXX/ctor-dtor-alias.cpp
  test/CodeGenCXX/destructors.cpp
  test/CodeGenCXX/dllexport-alias.cpp
  test/CodeGenCXX/dllexport.cpp
  test/CodeGenCXX/microsoft-abi-structors-alias.cpp
  test/CodeGenCXX/virtual-bases.cpp
  test/CodeGenCXX/virtual-destructor-calls.cpp

Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1193,7 +1193,6 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
-  bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
 
   llvm::GlobalValue *GetGlobalValue(StringRef Ref);
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -3693,6 +3693,9 @@
   // Create the alias with no name.
   auto *Alias = llvm::GlobalAlias::create(Linkage, "", Aliasee);
 
+  // Constructors and destructors are always unnamed_addr.
+  Alias->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+
   // Switch any previous uses to the alias.
   if (Entry) {
 assert(Entry->getType() == Aliasee->getType() &&
Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -109,17 +109,8 @@
   D->getType()->getAs()->getCallConv())
 return true;
 
-  return TryEmitDefinitionAsAlias(GlobalDecl(D, Dtor_Base),
-  GlobalDecl(BaseD, Dtor_Base));
-}
-
-/// Try to emit a definition as a global alias for another definition.
-/// If \p InEveryTU is true, we know that an equivalent alias can be produced
-/// in every translation unit.
-bool CodeGenModule::TryEmitDefinitionAsAlias(GlobalDecl AliasDecl,
- GlobalDecl TargetDecl) {
-  if (!getCodeGenOpts().CXXCtorDtorAliases)
-return true;
+  GlobalDecl AliasDecl(D, Dtor_Base);
+  GlobalDecl TargetDecl(BaseD, Dtor_Base);
 
   // The alias will use the linkage of the referent.  If we can't
   // support aliases with that linkage, fail.
@@ -193,6 +184,9 @@
   auto *Alias = llvm::GlobalAlias::create(AliasValueType, 0, Linkage, "",
   Aliasee, &getModule());
 
+  // Destructors are always unnamed_addr.
+  Alias->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+
   // Switch any previous uses to the alias.
   if (Entry) {
 assert(Entry->getType() == AliasType &&
Index: test/CodeGenCXX/microsoft-abi-structors-alias.cpp
===
--- test/CodeGenCXX/microsoft-abi-structors-alias.cpp
+++ test/CodeGenCXX/microsoft-abi-structors-alias.cpp
@@ -22,7 +22,7 @@
 void foo() {
   B b;
 }
-// CHECK-DAG: @"??1B@test2@@UAE@XZ" = dso_local alias void (%"struct.test2::B"*), bitcast (void (%"struct.test2::A"*)* @"??1A@test2@@UAE@XZ" to void (%"struct.test2::B"*)*)
+// CHECK-DAG: @"??1B@test2@@UAE@XZ" = dso_local unnamed_addr alias void (%"struct.test2::B"*), bitcast (void (%"struct.test2::A"*)* @"??1A@test2@@UAE@XZ" to void (%"struct.test2::B"*)*)
 }
 
 namespace test3 {
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -641,7 +641,7 @@
   A::~A() { }
   B::~B() { }
   // Emit a alias definition of B's constructor.
-  // M32-DAG: @"??1B@UseDtorAlias@@QAE@XZ" = dso_local dllexport alias {{.*}} @"??1A@UseDtorAlias@@QAE@XZ"
+  // M32-DAG: @"??1B@UseDtorAlias@@QAE@XZ" = dso_local dllexport unnamed_addr alias {{.*}} @"??1A@UseDtorAlias@@QAE@XZ"
 }
 
 struct __declspec(dllexport) DefaultedCtorsDtors {
Index: test/CodeGenCXX/dllexport-alias.cpp
===
--- test/CodeGenCXX/dllexport-alias.cpp
+++ test/CodeGenCXX/dllexport-alias.cpp
@@ -14,5 +14,5 @@
 
 A::~A() {}
 
-// CHECK: @_ZN1AC1Ev = dso_local dllexport alias void (%class.A*), void (%class.A*)* @_ZN1AC2Ev
-// CHECK: @_ZN1AD1Ev = dso_local dllexport alias void (%class.A*), void (%class.A*)* @_ZN1AD2Ev
+// CHECK: @_ZN1AC1Ev = dso_local dllexport unnamed_addr alias void (%class.A*), void (%class.A*)* @_ZN1AC2Ev
+// CHECK: @_ZN1AD1Ev = dso_local dllexport unnamed_addr alias void (%class.A*), void (%class.A*)* @_ZN1AD2Ev
Index: test/CodeGenCXX/virtual-destructor-calls.cpp

[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:423
+llvm::sys::path::native(Dwo_Dir, DwoDir);
+llvm::sys::path::append(DwoDir, Twine(Output.getFilename()) + "_dwo");
+CmdArgs.push_back(

I think that if I pass `-gsplit-dwarf=/path/to/foo` I would expect the dwo 
directory to be named `/path/to/foo`, not `/path/to/foo/something_dwo`.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:428
+
+  if (Args.hasArg(options::OPT_gsplit_dwarf)) {
+if (!Args.getLastArg(options::OPT_gsplit_dwarf_EQ)) {

If you make this `else if (Args.hasArg(options::OPT_gsplit_dwarf)) {` you 
wouldn't need the if on line 429.


https://reviews.llvm.org/D44788



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

[PATCH] D48454: Ignore blacklist when generating __cfi_check_fail.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

__cfi_check_fail certainly seems like a special case in that its behaviour is 
controlled only by flags and not the blacklist.

Maybe a simpler fix would be to add this to the top of `EmitCfiCheckFail`?

  SanOpts = CGM.getLangOpts().Sanitize;


https://reviews.llvm.org/D48454



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

[PATCH] D48454: Ignore blacklist when generating __cfi_check_fail.

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D48454



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

[PATCH] D52857: [clang-query] Add non-exclusive output API

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I'm not really involved in clang-query development these days, so this is more 
of a drive-by comment. It wasn't really obvious to me what the wording of the 
help text for `set enable-output` meant:

> Set whether to enable  content non-exclusively.

I figured out from the name of the option and from reading the source code that 
it just means "Enable output in format ." This seems confusing to me 
because I would expect `set foo bar` to set the value of a variable `foo` to 
`bar`, which doesn't match what this command does. What do you think about 
spelling it as something like `enable output `?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52857



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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Peter Collingbourne via Phabricator via cfe-commits]

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

The reason why LTO unit is always enabled is so that you can link translation 
units compiled with `-fsanitize=cfi` and/or `-fwhole-program-vtables` against 
translation units compiled without CFI/WPD. With this change we will see 
miscompiles in the translation units compiled with CFI/WPD if they use vtables 
in the translation units compiled without CFI/WPD. If we really need this 
option I think it should be an opt out.


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote:

> In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D53524#1271357, @pcc wrote:
> >
> > > The reason why LTO unit is always enabled is so that you can link 
> > > translation units compiled with `-fsanitize=cfi` and/or 
> > > `-fwhole-program-vtables` against translation units compiled without 
> > > CFI/WPD. With this change we will see miscompiles in the translation 
> > > units compiled with CFI/WPD if they use vtables in the translation units 
> > > compiled without CFI/WPD. If we really need this option I think it should 
> > > be an opt out.
> >
> >
> > Is there an important use case for support thing mixing and matching? The 
> > issue is that it comes at a cost to all ThinLTO compiles for codes with 
> > vtables by requiring them all to process IR during the thin link.
>
>
> Ping on the question of why this mode needs to be default. If it was a matter 
> of a few percent overhead that would be one thing, but we're talking a *huge* 
> overhead (as noted off-patch for my app I'm seeing >20x thin link time 
> currently, and with improvements to the hashing to always get successful 
> splitting we could potentially get down to closer to 2x - still a big 
> overhead). This kind of overhead should be opt-in. The average ThinLTO user 
> is not going to realize they are paying a big overhead because CFI is always 
> pre-enabled.


Well, the intent was always that the overhead would be minimal, which is why 
things are set up the way that they are. But it doesn't sound like anyone is 
going to have the time to fully address the performance problems that you've 
seen any time soon, so maybe it would be fine to introduce the -flto-unit flag. 
I guess we can always change the flag so that it has no effect if/when the 
performance problem is addressed.

>> Can we detect that TUs compiled with -flto-unit are being mixed with those 
>> not built without -flto-unit at the thin link time and issue an error?
> 
> This would be doable pretty easily. E.g. add a flag at the index level that 
> the module would have been split but wasn't. Users who get the error and want 
> to support always-enabled CFI could opt in via -flto-unit.

Yes. I don't think we should make a change like this unless there is something 
like that in place, though. The documentation (LTOVisibility.rst) needs to be 
updated too.


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D53524#1279288, @tejohnson wrote:

> In https://reviews.llvm.org/D53524#1276038, @pcc wrote:
>
> > In https://reviews.llvm.org/D53524#1274505, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D53524#1271387, @tejohnson wrote:
> > >
> > > > In https://reviews.llvm.org/D53524#1271357, @pcc wrote:
> > > >
> > > > > The reason why LTO unit is always enabled is so that you can link 
> > > > > translation units compiled with `-fsanitize=cfi` and/or 
> > > > > `-fwhole-program-vtables` against translation units compiled without 
> > > > > CFI/WPD. With this change we will see miscompiles in the translation 
> > > > > units compiled with CFI/WPD if they use vtables in the translation 
> > > > > units compiled without CFI/WPD. If we really need this option I think 
> > > > > it should be an opt out.
> > > >
> > > >
> > > > Is there an important use case for support thing mixing and matching? 
> > > > The issue is that it comes at a cost to all ThinLTO compiles for codes 
> > > > with vtables by requiring them all to process IR during the thin link.
> > >
> > >
> > > Ping on the question of why this mode needs to be default. If it was a 
> > > matter of a few percent overhead that would be one thing, but we're 
> > > talking a *huge* overhead (as noted off-patch for my app I'm seeing >20x 
> > > thin link time currently, and with improvements to the hashing to always 
> > > get successful splitting we could potentially get down to closer to 2x - 
> > > still a big overhead). This kind of overhead should be opt-in. The 
> > > average ThinLTO user is not going to realize they are paying a big 
> > > overhead because CFI is always pre-enabled.
> >
> >
> > Well, the intent was always that the overhead would be minimal, which is 
> > why things are set up the way that they are. But it doesn't sound like 
> > anyone is going to have the time to fully address the performance problems 
> > that you've seen any time soon, so maybe it would be fine to introduce the 
> > -flto-unit flag. I guess we can always change the flag so that it has no 
> > effect if/when the performance problem is addressed.
>
>
> Just to clarify, since there is already a -flto-unit flag: it is currently a 
> cc1 flag, did you want it made into a driver option as well?


Yes, that's what I had in mind.

 Can we detect that TUs compiled with -flto-unit are being mixed with those 
 not built without -flto-unit at the thin link time and issue an error?
>>> 
>>> This would be doable pretty easily. E.g. add a flag at the index level that 
>>> the module would have been split but wasn't. Users who get the error and 
>>> want to support always-enabled CFI could opt in via -flto-unit.
>> 
>> Yes. I don't think we should make a change like this unless there is 
>> something like that in place, though. The documentation (LTOVisibility.rst) 
>> needs to be updated too.
> 
> Ok, let me work on that now and we can get that in before this one.




Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D38430: Enable -pie and --enable-new-dtags by default on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.
Herald added a subscriber: llvm-commits.



Comment at: cfe/trunk/lib/Driver/ToolChains/Linux.cpp:814
+bool Linux::isPIEDefault() const {
+  return (getTriple().isAndroid() && !getTriple().isAndroidVersionLT(16)) ||
+ getSanitizerArgs().requiresPIE();

Why only on API level >= 16? If I create an executable targeting an older API 
level, it should still work on higher API levels.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D38430



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

[PATCH] D38430: Enable -pie and --enable-new-dtags by default on Android.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: cfe/trunk/lib/Driver/ToolChains/Linux.cpp:814
+bool Linux::isPIEDefault() const {
+  return (getTriple().isAndroid() && !getTriple().isAndroidVersionLT(16)) ||
+ getSanitizerArgs().requiresPIE();

eugenis wrote:
> pcc wrote:
> > Why only on API level >= 16? If I create an executable targeting an older 
> > API level, it should still work on higher API levels.
> Because it needs to work on lower API levels, too. I think at some point PIE 
> was actually not supported, so there is no good default that works for 
> everyone.
I see. Looking at the tags in which 
https://github.com/aosp-mirror/platform_bionic/commit/d9ad62343c2db6b66a5fa597c9b20a6faabd7a9a
 was present, support was indeed added in Jelly Bean, which was API level 16. 
So this is correct.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D38430



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

[PATCH] D54604: Automatic variable initialization

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: cfe/trunk/include/clang/Driver/Options.td:1657
+  " | pattern">, Values<"uninitialized,pattern">;
+def enable_trivial_var_init_zero : Joined<["-"], 
"enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
+  Flags<[CC1Option]>,

I noticed that this introduces a parsing ambiguity on the command line: the 
flag `-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang` 
can be interpreted either as this flag or as `-e 
nable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang`, 
meaning "tell the linker to use that long symbol name as the program's entry 
point". Worth fixing (e.g. by renaming to `--enable-...`)? It's probably a good 
idea to rename it every so often just to keep people on their toes anyway.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54604



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

[PATCH] D54604: Automatic variable initialization

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: cfe/trunk/include/clang/Driver/Options.td:1657
+  " | pattern">, Values<"uninitialized,pattern">;
+def enable_trivial_var_init_zero : Joined<["-"], 
"enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
+  Flags<[CC1Option]>,

jfb wrote:
> pcc wrote:
> > I noticed that this introduces a parsing ambiguity on the command line: the 
> > flag 
> > `-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang` 
> > can be interpreted either as this flag or as `-e 
> > nable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang`, 
> > meaning "tell the linker to use that long symbol name as the program's 
> > entry point". Worth fixing (e.g. by renaming to `--enable-...`)? It's 
> > probably a good idea to rename it every so often just to keep people on 
> > their toes anyway.
> Don't we have the same issue with:
> 
> ```
> 
> include/clang/Driver/CC1Options.td:def emit_html : Flag<["-"], "emit-html">,
> include/clang/Driver/CC1Options.td:def emit_module : Flag<["-"], 
> "emit-module">,
> include/clang/Driver/CC1Options.td:def emit_module_interface : Flag<["-"], 
> "emit-module-interface">,
> include/clang/Driver/CC1Options.td:def emit_header_module : Flag<["-"], 
> "emit-header-module">,
> include/clang/Driver/CC1Options.td:def emit_pch : Flag<["-"], "emit-pch">,
> include/clang/Driver/CC1Options.td:def emit_llvm_bc : Flag<["-"], 
> "emit-llvm-bc">,
> include/clang/Driver/CC1Options.td:def emit_llvm_only : Flag<["-"], 
> "emit-llvm-only">,
> include/clang/Driver/CC1Options.td:def emit_codegen_only : Flag<["-"], 
> "emit-codegen-only">,
> include/clang/Driver/CC1Options.td:def emit_obj : Flag<["-"], "emit-obj">,
> include/clang/Driver/CC1Options.td:def emit_llvm_uselists : Flag<["-"], 
> "emit-llvm-uselists">,
> include/clang/Driver/CC1Options.td:def enable_split_dwarf : Flag<["-"], 
> "enable-split-dwarf">,
> include/clang/Driver/CC1Options.td:def enable_split_dwarf_EQ : Joined<["-"], 
> "enable-split-dwarf=">,
> include/clang/Driver/CC1Options.td:def error_on_deserialized_pch_decl : 
> Separate<["-"], "error-on-deserialized-decl">,
> include/clang/Driver/CC1Options.td:def error_on_deserialized_pch_decl_EQ : 
> Joined<["-"], "error-on-deserialized-decl=">,
> include/clang/Driver/Options.td:def emit_ast : Flag<["-"], "emit-ast">,
> include/clang/Driver/Options.td:def emit_llvm : Flag<["-"], "emit-llvm">, 
> Flags<[CC1Option]>, Group,
> include/clang/Driver/Options.td:def exported__symbols__list : Separate<["-"], 
> "exported_symbols_list">;
> include/clang/Driver/Options.td:def e : JoinedOrSeparate<["-"], "e">, 
> Group;
> include/clang/Driver/Options.td:def enable_trivial_var_init_zero : 
> Joined<["-"], 
> "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
> ```
> 
> ?
Just the ones in `Options.td`, I believe (since this is a driver-only flag).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54604



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

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: eugenis.
Herald added subscribers: llvm-commits, cryptoad.

Repository:
  rL LLVM

https://reviews.llvm.org/D54330

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: 
"-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346526: Driver: Make -fsanitize=shadow-call-stack compatible 
with -fsanitize-minimal… (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54330?vs=173367&id=173368#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54330

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/Driver/fsanitize.c


Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {
Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: 
"-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO


Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {
Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: docs/LTOVisibility.rst:9
 unit's *LTO unit* is the subset of the linkage unit that is linked together
-using link-time optimization; in the case where LTO is not being used, the
-linkage unit's LTO unit is empty. Each linkage unit has only a single LTO unit.
+using link-time optimization; in the case where LTO units are not being used,
+the linkage unit's LTO unit is empty. Each linkage unit has only a single LTO

It's a little confusing to talk about "LTO units" as a property of a 
translation unit when there is only one LTO unit per linkage unit. I think this 
should say that an LTO unit is the subset of the linkage unit compiled with 
certain flags. Then in the rest of the document you can talk about translation 
units that are either part of or not part of the LTO unit.


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In https://reviews.llvm.org/D46403#1291697, @filcab wrote:

> Sorry to ressurect this review, but I have a few questions:
>
> - What kind of functions fail?
> - Are there bugzillas to track these?
> - How can a compiler expect to have blacklists for "all" its CFI clients? 
> (I'm ok with having a default for "most-used, well-known, problematic 
> functions", e.g: functions in libstdc++ or libc++)


The default blacklist should contain problematic functions in the stdlibs that 
we don't control (i.e. libstdc++ and the MSVC stdlib). In libc++, we blacklist 
functions directly in the source code using the `no_sanitize` attribute. In 
general the blacklist entries for the stdlib are necessary because the standard 
requires that functions have a particular signature which requires the 
implementation to make what is considered to be a bad cast by CFI, so there 
isn't a bug as such.

> - clang/compiler-rt don't seem to have a default blacklist, what should the 
> contents be?

There is a default blacklist, see 
http://llvm-cs.pcc.me.uk/projects/compiler-rt/lib/cfi/cfi_blacklist.txt
Its contents are as mentioned above.

It should be installed by default, is that not happening for you?

> This patch seems to be saying "There are some well-known functions that 
> should never be instrumented with CFI, I'll provide a list of names", which 
> doesn't really seem possible to do in general, for "most" CFI-toolchain 
> clients. As I see it, each client will need to have their own blacklist, and 
> provide it as an argument if needed.

Right, if the client needs a blacklist it should be passed on the command line. 
More importantly, it should be applied in addition to the standard one so that 
the user doesn't have to repeat the stdlib blacklist in their blacklist (which 
was the situation before we added support for multiple blacklists).

> Which brings us to:
> 
> - If I pass `-fsanitize-blacklist=my_blacklist.txt` I still get an error. 
> This is not ideal, as I might be working on the blacklist to include in my 
> toolchain/program.
> 
>   I don't think we should have an error that is default enabled for this 
> issue. At most a warning (+ `-Werror`) if there was no blacklist passed in 
> nor found in the toolchain resource directory.

Clang prints this error if the blacklist is missing from the resource 
directory, which means that Clang was not installed properly. This is 
intentional as it prevents us from silently not applying the standard blacklist 
in that case.

If you are working on the toolchain blacklist, that is a very special case and 
there are plenty of ways around this issue. For example you can edit 
`lib/cfi/cfi_blacklist.txt` in place and use `ninja` to install it.


Repository:
  rC Clang

https://reviews.llvm.org/D46403



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

[PATCH] D54453: Remove myself as owner of clang-query.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: klimek, alexfh.
Herald added a subscriber: llvm-commits.

I haven't been involved with the project for years, so it's probably
best for someone else to be the code owner.


Repository:
  rL LLVM

https://reviews.llvm.org/D54453

Files:
  clang-tools-extra/CODE_OWNERS.TXT


Index: clang-tools-extra/CODE_OWNERS.TXT
===
--- clang-tools-extra/CODE_OWNERS.TXT
+++ clang-tools-extra/CODE_OWNERS.TXT
@@ -8,10 +8,6 @@
 (W), PGP key ID and fingerprint (P), description (D), and snail-mail address
 (S).
 
-N: Peter Collingbourne
-E: pe...@pcc.me.uk
-D: clang-query
-
 N: Manuel Klimek
 E: kli...@google.com
 D: clang-rename, all parts of clang-tools-extra not covered by someone else


Index: clang-tools-extra/CODE_OWNERS.TXT
===
--- clang-tools-extra/CODE_OWNERS.TXT
+++ clang-tools-extra/CODE_OWNERS.TXT
@@ -8,10 +8,6 @@
 (W), PGP key ID and fingerprint (P), description (D), and snail-mail address
 (S).
 
-N: Peter Collingbourne
-E: pe...@pcc.me.uk
-D: clang-query
-
 N: Manuel Klimek
 E: kli...@google.com
 D: clang-rename, all parts of clang-tools-extra not covered by someone else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54453: Remove myself as owner of clang-query.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE346998: Remove myself as owner of clang-query. (authored 
by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54453?vs=173776&id=174282#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54453

Files:
  CODE_OWNERS.TXT


Index: CODE_OWNERS.TXT
===
--- CODE_OWNERS.TXT
+++ CODE_OWNERS.TXT
@@ -12,10 +12,6 @@
 E: aa...@aaronballman.com
 D: clang-query
 
-N: Peter Collingbourne
-E: pe...@pcc.me.uk
-D: clang-query
-
 N: Manuel Klimek
 E: kli...@google.com
 D: clang-rename, all parts of clang-tools-extra not covered by someone else


Index: CODE_OWNERS.TXT
===
--- CODE_OWNERS.TXT
+++ CODE_OWNERS.TXT
@@ -12,10 +12,6 @@
 E: aa...@aaronballman.com
 D: clang-query
 
-N: Peter Collingbourne
-E: pe...@pcc.me.uk
-D: clang-query
-
 N: Manuel Klimek
 E: kli...@google.com
 D: clang-rename, all parts of clang-tools-extra not covered by someone else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54735: Driver: SCS is compatible with every other sanitizer.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: eugenis.
Herald added a subscriber: llvm-commits.

Because SCS relies on system-provided runtime support, we can use it
together with any other sanitizer simply by linking the runtime for
the other sanitizer.


Repository:
  rL LLVM

https://reviews.llvm.org/D54735

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/sanitizer-ld.c


Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -588,7 +588,7 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -fsanitize=safe-stack -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-SAFESTACK %s
-// CHECK-SHADOWCALLSTACK-SAFESTACK: error: invalid argument 
'-fsanitize=shadow-call-stack' not allowed with '-fsanitize=safe-stack'
+// CHECK-SHADOWCALLSTACK-SAFESTACK-NOT: error:
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -376,12 +376,9 @@
 KernelAddress | Efficiency),
   std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
 KernelAddress | Efficiency),
-  std::make_pair(ShadowCallStack, Address | HWAddress | Leak | Thread |
-  Memory | KernelAddress | Efficiency |
-  SafeStack),
   std::make_pair(KernelHWAddress, Address | HWAddress | Leak | Thread |
   Memory | KernelAddress | Efficiency |
-  SafeStack | ShadowCallStack),
+  SafeStack),
   std::make_pair(KernelMemory, Address | HWAddress | Leak | Thread |
Memory | KernelAddress | Efficiency |
Scudo | SafeStack)};


Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -588,7 +588,7 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -fsanitize=safe-stack -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-SAFESTACK %s
-// CHECK-SHADOWCALLSTACK-SAFESTACK: error: invalid argument '-fsanitize=shadow-call-stack' not allowed with '-fsanitize=safe-stack'
+// CHECK-SHADOWCALLSTACK-SAFESTACK-NOT: error:
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -376,12 +376,9 @@
 KernelAddress | Efficiency),
   std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
 KernelAddress | Efficiency),
-  std::make_pair(ShadowCallStack, Address | HWAddress | Leak | Thread |
-  Memory | KernelAddress | Efficiency |
-  SafeStack),
   std::make_pair(KernelHWAddress, Address | HWAddress | Leak | Thread |
   Memory | KernelAddress | Efficiency |
-  SafeStack | ShadowCallStack),
+  SafeStack),
   std::make_pair(KernelMemory, Address | HWAddress | Leak | Thread |
Memory | KernelAddress | Efficiency |
Scudo | SafeStack)};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54735: Driver: SCS is compatible with every other sanitizer.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC347282: Driver: SCS is compatible with every other 
sanitizer. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54735?vs=174709&id=174714#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54735

Files:
  lib/Driver/SanitizerArgs.cpp
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -588,7 +588,7 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -fsanitize=safe-stack -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-SAFESTACK %s
-// CHECK-SHADOWCALLSTACK-SAFESTACK: error: invalid argument 
'-fsanitize=shadow-call-stack' not allowed with '-fsanitize=safe-stack'
+// CHECK-SHADOWCALLSTACK-SAFESTACK-NOT: error:
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -376,12 +376,9 @@
 KernelAddress | Efficiency),
   std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
 KernelAddress | Efficiency),
-  std::make_pair(ShadowCallStack, Address | HWAddress | Leak | Thread |
-  Memory | KernelAddress | Efficiency |
-  SafeStack),
   std::make_pair(KernelHWAddress, Address | HWAddress | Leak | Thread |
   Memory | KernelAddress | Efficiency |
-  SafeStack | ShadowCallStack),
+  SafeStack),
   std::make_pair(KernelMemory, Address | HWAddress | Leak | Thread |
Memory | KernelAddress | Efficiency |
Scudo | SafeStack)};


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -588,7 +588,7 @@
 // RUN: %clang -fsanitize=shadow-call-stack %s -### -o %t.o 2>&1 \
 // RUN: -fsanitize=safe-stack -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-SAFESTACK %s
-// CHECK-SHADOWCALLSTACK-SAFESTACK: error: invalid argument '-fsanitize=shadow-call-stack' not allowed with '-fsanitize=safe-stack'
+// CHECK-SHADOWCALLSTACK-SAFESTACK-NOT: error:
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -376,12 +376,9 @@
 KernelAddress | Efficiency),
   std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
 KernelAddress | Efficiency),
-  std::make_pair(ShadowCallStack, Address | HWAddress | Leak | Thread |
-  Memory | KernelAddress | Efficiency |
-  SafeStack),
   std::make_pair(KernelHWAddress, Address | HWAddress | Leak | Thread |
   Memory | KernelAddress | Efficiency |
-  SafeStack | ShadowCallStack),
+  SafeStack),
   std::make_pair(KernelMemory, Address | HWAddress | Leak | Thread |
Memory | KernelAddress | Efficiency |
Scudo | SafeStack)};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54805: [Driver] Use --push/pop-state with Sanitizer link deps

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Unfortunately it looks like the Android NDK uses some ancient version of gold 
that doesn't support `--push-state`, so we probably can't rely on being able to 
use it.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/17287/steps/run%20lit%20tests%20%5Bi686%2Ffugu-userdebug%2FN2G48C%5D/logs/stdio
As an alternative solution, could we add these flags at the start of the linker 
command line? That way, we're guaranteed that the linker will be in the 
`--no-as-needed` state.


Repository:
  rC Clang

https://reviews.llvm.org/D54805



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

[PATCH] D55159: Lex: Fix bug in __BASE_FILE__ implementation.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: rsmith.
Herald added a subscriber: llvm-commits.

__BASE_FILE__ previously expanded to "" instead of the name
of the main source file in files included using -include.

Also added test coverage for __BASE_FILE__ since there doesn't seem
to be any.


Repository:
  rL LLVM

https://reviews.llvm.org/D55159

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/Inputs/base_file.h
  clang/test/Preprocessor/base_file.c


Index: clang/test/Preprocessor/base_file.c
===
--- /dev/null
+++ clang/test/Preprocessor/base_file.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -include %S/Inputs/base_file.h -E %s | FileCheck %s
+
+// CHECK: {{[\\/]}}base_file.c"
+
+// CHECK: {{[\\/]}}base_file.c"
+#include "Inputs/base_file.h"
+
+// CHECK: {{[\\/]}}base_file.c"
+__BASE_FILE__
Index: clang/test/Preprocessor/Inputs/base_file.h
===
--- /dev/null
+++ clang/test/Preprocessor/Inputs/base_file.h
@@ -0,0 +1 @@
+__BASE_FILE__
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1505,24 +1505,11 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
 
-// __BASE_FILE__ is a GNU extension that returns the top of the presumed
-// #include stack instead of the current file.
-if (II == Ident__BASE_FILE__ && PLoc.isValid()) {
-  SourceLocation NextLoc = PLoc.getIncludeLoc();
-  while (NextLoc.isValid()) {
-PLoc = SourceMgr.getPresumedLoc(NextLoc);
-if (PLoc.isInvalid())
-  break;
-
-NextLoc = PLoc.getIncludeLoc();
-  }
-}
-
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
@@ -1531,6 +1518,12 @@
   OS << '"' << FN << '"';
 }
 Tok.setKind(tok::string_literal);
+  } else if (II == Ident__BASE_FILE__) {
+SmallString<128> FN =
+SourceMgr.getFileEntryForID(SourceMgr.getMainFileID())->getName();
+Lexer::Stringify(FN);
+OS << '"' << FN << '"';
+Tok.setKind(tok::string_literal);
   } else if (II == Ident__DATE__) {
 Diag(Tok.getLocation(), diag::warn_pp_date_time);
 if (!DATELoc.isValid())


Index: clang/test/Preprocessor/base_file.c
===
--- /dev/null
+++ clang/test/Preprocessor/base_file.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -include %S/Inputs/base_file.h -E %s | FileCheck %s
+
+// CHECK: {{[\\/]}}base_file.c"
+
+// CHECK: {{[\\/]}}base_file.c"
+#include "Inputs/base_file.h"
+
+// CHECK: {{[\\/]}}base_file.c"
+__BASE_FILE__
Index: clang/test/Preprocessor/Inputs/base_file.h
===
--- /dev/null
+++ clang/test/Preprocessor/Inputs/base_file.h
@@ -0,0 +1 @@
+__BASE_FILE__
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1505,24 +1505,11 @@
 // __LINE__ expands to a simple numeric value.
 OS << (PLoc.isValid()? PLoc.getLine() : 1);
 Tok.setKind(tok::numeric_constant);
-  } else if (II == Ident__FILE__ || II == Ident__BASE_FILE__) {
+  } else if (II == Ident__FILE__) {
 // C99 6.10.8: "__FILE__: The presumed name of the current source file (a
 // character string literal)". This can be affected by #line.
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
 
-// __BASE_FILE__ is a GNU extension that returns the top of the presumed
-// #include stack instead of the current file.
-if (II == Ident__BASE_FILE__ && PLoc.isValid()) {
-  SourceLocation NextLoc = PLoc.getIncludeLoc();
-  while (NextLoc.isValid()) {
-PLoc = SourceMgr.getPresumedLoc(NextLoc);
-if (PLoc.isInvalid())
-  break;
-
-NextLoc = PLoc.getIncludeLoc();
-  }
-}
-
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
 SmallString<128> FN;
 if (PLoc.isValid()) {
@@ -1531,6 +1518,12 @@
   OS << '"' << FN << '"';
 }
 Tok.setKind(tok::string_literal);
+  } else if (II == Ident__BASE_FILE__) {
+SmallString<128> FN =
+SourceMgr.getFileEntryForID(SourceMgr.getMainFileID())->getName();
+Lexer::Stringify(FN);
+OS << '"' << FN << '"';
+Tok.setKind(tok::string_literal);
   } else if (II == Ident__DATE__) {
 Diag

[PATCH] D55229: [COFF, ARM64] Make -flto-visibility-public-std a driver and cc1 flag

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

The reason why it has that name is that it was originally added to support the 
logic here: http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGVTables.cpp#991

It looks like it was later repurposed as a "was /MT passed to the driver" flag 
when the logic was added to mark runtime functions as dllimport depending on 
the value of the flag.

It certainly seems to make sense to rename it since the name makes no sense as 
a driver flag and is now being used for other purposes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55229



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

[PATCH] D55246: AST: Relax an assertion in constexpr constructor evaluation.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: rsmith.
Herald added a subscriber: llvm-commits.

It's possible for the base type specified in a constructor to be
qualified via sugar, so the existing assertion was too strict. Relax
it so that we only check that the unqualified types are the same.


Repository:
  rL LLVM

https://reviews.llvm.org/D55246

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2220,3 +2220,15 @@
   constexpr int *q = (&n + 1) - (unsigned __int128)-1; // expected-error 
{{constant expression}} expected-note {{cannot refer to element -3402}}
   constexpr int *r = &(&n + 1)[(unsigned __int128)-1]; // expected-error 
{{constant expression}} expected-note {{cannot refer to element 3402}}
 }
+
+namespace QualifiedBaseType {
+  template 
+  struct A : T {
+  public:
+constexpr A() : T() {}
+  };
+
+  struct S {};
+
+  A a;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4542,7 +4542,7 @@
   // Non-virtual base classes are initialized in the order in the class
   // definition. We have already checked for virtual base classes.
   assert(!BaseIt->isVirtual() && "virtual base for literal type");
-  assert(Info.Ctx.hasSameType(BaseIt->getType(), BaseType) &&
+  assert(Info.Ctx.hasSameUnqualifiedType(BaseIt->getType(), BaseType) &&
  "base class initializers not in expected order");
   ++BaseIt;
 #endif


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2220,3 +2220,15 @@
   constexpr int *q = (&n + 1) - (unsigned __int128)-1; // expected-error {{constant expression}} expected-note {{cannot refer to element -3402}}
   constexpr int *r = &(&n + 1)[(unsigned __int128)-1]; // expected-error {{constant expression}} expected-note {{cannot refer to element 3402}}
 }
+
+namespace QualifiedBaseType {
+  template 
+  struct A : T {
+  public:
+constexpr A() : T() {}
+  };
+
+  struct S {};
+
+  A a;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4542,7 +4542,7 @@
   // Non-virtual base classes are initialized in the order in the class
   // definition. We have already checked for virtual base classes.
   assert(!BaseIt->isVirtual() && "virtual base for literal type");
-  assert(Info.Ctx.hasSameType(BaseIt->getType(), BaseType) &&
+  assert(Info.Ctx.hasSameUnqualifiedType(BaseIt->getType(), BaseType) &&
  "base class initializers not in expected order");
   ++BaseIt;
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54604: Automatic variable initialization

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: include/clang/Driver/ToolChain.h:355
+  virtual LangOptions::TrivialAutoVarInitKind
+  GetDefaultTrivialAutoVarInit() const {
+return LangOptions::TrivialAutoVarInitKind::Uninitialized;

I wouldn't introduce this function because nothing is overriding it (for now, 
at least).



Comment at: lib/CodeGen/CGDecl.cpp:968
+  // The following value is a guaranteed unmappable pointer value and has a
+  // reperated byte-pattern which makes it easier to synthesize. We use it for
+  // pointers as well as integers so that aggregates are likely to be

*repeated



Comment at: lib/CodeGen/CGDecl.cpp:977
+  constexpr uint32_t SmallValue = 0x00AA;
+  if (Ty->isIntOrIntVectorTy()) {
+unsigned BitWidth = llvm::cast(

Do you have test coverage for vectors?



Comment at: lib/CodeGen/CGDecl.cpp:978
+  if (Ty->isIntOrIntVectorTy()) {
+unsigned BitWidth = llvm::cast(
+Ty->isVectorTy() ? Ty->getVectorElementType() : Ty)

Is it necessary to qualify `cast` with `llvm::`? It doesn't seem to be 
qualified elsewhere in this file. (Same below.)



Comment at: lib/CodeGen/CGDecl.cpp:1145
+  if (Ty->isStructTy() || Ty->isArrayTy() || Ty->isVectorTy()) {
+for (unsigned Op = 0, NumOp = constant->getNumOperands(); Op != NumOp;
+ ++Op) {

Maybe use `User::operands()` to enumerate operands here?



Comment at: lib/CodeGen/CGDecl.cpp:1631
+  auto initializeWhatIsTechnicallyUninitialized = [&]() {
+if (trivialAutoVarInit !=
+LangOptions::TrivialAutoVarInitKind::Uninitialized) {

Is it possible to rewrite this function using early returns to avoid all the 
indentation below for the VLA case? I'd also put the code for non-VLAs first to 
make this function easier to understand (on my first reading I somehow got the 
impression that this function is *only* handling VLAs). I'm thinking maybe 
something like:

```
if (trivialAutoVarInit == LangOptions::TrivialAutoVarInitKind::Uninitialized)
  return;

if (!getContext().getTypeSizeInChars(type).isZero()) {
  // handle non-VLAs
}

auto *VlaType = 
dyn_cast_or_null(getContext().getAsArrayType(type));
if (!VlaType)
  return;

// handle VLAs
```



Comment at: lib/CodeGen/CGDecl.cpp:1673
+llvm::BasicBlock *ContBB = createBasicBlock("vla-init.cont");
+EmitBlock(LoopBB);
+llvm::PHINode *Cur =

I think we need to check that the size is non-zero here. Otherwise we're going 
to clobber whatever comes after the VLA if its size is zero (I know that C 
forbids zero-sized VLAs, but I believe we support them as a GNU extension).



Comment at: test/CodeGenCXX/trivial-auto-var-init.cpp:5
+
+// PATTERN-NOT: undef
+// ZERO-NOT: undef

Since this is your only test involving structs and arrays, you probably want to 
test them beyond the fact that they don't contain undef. I also believe that 
this is testing for the absence of `undef` before the first label and not the 
total absence of `undef` in the output.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54604



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

[PATCH] D59034: Delete x86_64 ShadowCallStack support

[Peter Collingbourne via Phabricator via cfe-commits]

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

Thanks for cleaning this up.




Comment at: clang/docs/ShadowCallStack.rst:23
+to have critical performance and security deficiencies--it was removed in
+LLVM 9.0.
 

Should we keep a link to the Clang 7.0.1 docs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59034



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

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

This needs a test under `test/CodeGen` at least.




Comment at: clang/include/clang/AST/RecordFieldReorganizer.h:54
+  std::seed_seq Seq;
+  std::default_random_engine rng;
+};

I don't think we can use `default_random_engine` for this because the behaviour 
would need to be consistent between C++ standard library implementations, and 
the behaviour of `default_random_engine` is implementation defined. Similarly, 
I don't think that we can use `std::shuffle` (see note in 
https://en.cppreference.com/w/cpp/algorithm/random_shuffle ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added reviewers: jfb, glider.
Herald added a project: clang.

Otherwise the object may have an incorrect size due to tail padding.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59446

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp


Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -85,6 +85,8 @@
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private 
unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr 
constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private 
unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } 
{ i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { 
i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] c"\AA\AA\AA", i32 13371338 } }, align 4
 // ZERO: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] zeroinitializer, i32 13371338 } }, align 4
@@ -778,6 +780,10 @@
 // CHECK-NEXT:  call void 
@llvm.memcpy{{.*}}({{.*}}@__const.test_paddedpackedarray_custom.custom
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
+TEST_UNINIT(unpackedinpacked, unpackedinpacked);
+// CHECK-LABEL: @test_unpackedinpacked_uninit()
+// PATTERN-O0:  call void @llvm.memcpy{{.*}}, i64 9, i1 false)
+
 TEST_UNINIT(paddednested, paddednested);
 // CHECK-LABEL: @test_paddednested_uninit()
 // CHECK:   %uninit = alloca %struct.paddednested, align
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1113,7 +1113,7 @@
   }
   if (NestedIntact && Values.size() == STy->getNumElements())
 return constant;
-  return llvm::ConstantStruct::getAnon(Values);
+  return llvm::ConstantStruct::getAnon(Values, STy->isPacked());
 }
 
 /// Replace all padding bytes in a given constant with either a pattern byte or


Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -85,6 +85,8 @@
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] [%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] [%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 } { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, { i8, [3 x i8], i32 } { i8 43, [3 x i8] c"\AA\AA\AA", i32 13371338 } }, align 4
 // ZERO: @__const.test_paddednested_custom.custom = private unnamed_addr cons

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356328: CodeGen: Preserve packed attribute in 
constStructWithPadding. (authored by pcc, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59446?vs=190942&id=190990#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59446

Files:
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/test/CodeGenCXX/auto-var-init.cpp


Index: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
@@ -85,6 +85,8 @@
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private 
unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr 
constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private 
unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } 
{ i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { 
i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] c"\AA\AA\AA", i32 13371338 } }, align 4
 // ZERO: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] zeroinitializer, i32 13371338 } }, align 4
@@ -778,6 +780,10 @@
 // CHECK-NEXT:  call void 
@llvm.memcpy{{.*}}({{.*}}@__const.test_paddedpackedarray_custom.custom
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
+TEST_UNINIT(unpackedinpacked, unpackedinpacked);
+// CHECK-LABEL: @test_unpackedinpacked_uninit()
+// PATTERN-O0:  call void @llvm.memcpy{{.*}}, i64 9, i1 false)
+
 TEST_UNINIT(paddednested, paddednested);
 // CHECK-LABEL: @test_paddednested_uninit()
 // CHECK:   %uninit = alloca %struct.paddednested, align
Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -1113,7 +1113,7 @@
   }
   if (NestedIntact && Values.size() == STy->getNumElements())
 return constant;
-  return llvm::ConstantStruct::getAnon(Values);
+  return llvm::ConstantStruct::getAnon(Values, STy->isPacked());
 }
 
 /// Replace all padding bytes in a given constant with either a pattern byte or


Index: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
@@ -85,6 +85,8 @@
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] [%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] [%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr cons