[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

[Alvin Wong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8cd90fd1a823: [X86][MC] Fix parsing Intel syntax indirect 
branch with symbol only (authored by alvinhochun).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/test/MC/X86/intel-syntax-branch.s

Index: llvm/test/MC/X86/intel-syntax-branch.s
===
--- /dev/null
+++ llvm/test/MC/X86/intel-syntax-branch.s
@@ -0,0 +1,71 @@
+// RUN: llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-32,CHECK
+// RUN: llvm-mc -triple x86_64-unknown-unknown --defsym X64=1 -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-64,CHECK
+
+// RUN: not llvm-mc -triple i686-unknown-unknown --defsym ERR=1 -x86-asm-syntax=intel %s 2>&1 | FileCheck %s --check-prefixes=ERR-32
+
+t0:
+call direct_branch
+jmp direct_branch
+// CHECK-LABEL: t0:
+// CHECK-64: callq direct_branch
+// CHECK-32: calll direct_branch
+// CHECK:jmp direct_branch
+
+t1:
+call [fn_ref]
+jmp [fn_ref]
+// CHECK-LABEL: t1:
+// CHECK-64: callq *fn_ref
+// CHECK-64: jmpq *fn_ref
+// CHECK-32: calll *fn_ref
+// CHECK-32: jmpl *fn_ref
+
+.ifdef X64
+
+  t2:
+  call qword ptr [fn_ref]
+  jmp qword ptr [fn_ref]
+  // CHECK-64-LABEL: t2:
+  // CHECK-64: callq *fn_ref
+  // CHECK-64: jmpq *fn_ref
+
+  t3:
+  call qword ptr [rip + fn_ref]
+  jmp qword ptr [rip + fn_ref]
+  // CHECK-64-LABEL: t3:
+  // CHECK-64: callq *fn_ref(%rip)
+  // CHECK-64: jmpq *fn_ref(%rip)
+
+.else
+
+  t4:
+  call dword ptr [fn_ref]
+  jmp dword ptr [fn_ref]
+  // CHECK-32-LABEL: t4:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t5:
+  call dword ptr fn_ref
+  jmp dword ptr fn_ref
+  // CHECK-32-LABEL: t5:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t6:
+  call dword ptr [offset fn_ref]
+  jmp dword ptr [offset fn_ref]
+  // CHECK-32-LABEL: t6:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+.ifdef ERR
+
+  call offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+  jmp offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+
+.endif
+
+.endif
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -433,6 +433,7 @@
 InlineAsmIdentifierInfo Info;
 short BracCount = 0;
 bool MemExpr = false;
+bool BracketUsed = false;
 bool OffsetOperator = false;
 bool AttachToOperandIdx = false;
 bool IsPIC = false;
@@ -455,6 +456,7 @@
 void addImm(int64_t imm) { Imm += imm; }
 short getBracCount() const { return BracCount; }
 bool isMemExpr() const { return MemExpr; }
+bool isBracketUsed() const { return BracketUsed; }
 bool isOffsetOperator() const { return OffsetOperator; }
 SMLoc getOffsetLoc() const { return OffsetOperatorLoc; }
 unsigned getBaseReg() const { return BaseReg; }
@@ -955,6 +957,7 @@
 break;
   }
   MemExpr = true;
+  BracketUsed = true;
   BracCount++;
   return false;
 }
@@ -2628,9 +2631,9 @@
   unsigned DefaultBaseReg = X86::NoRegister;
   bool MaybeDirectBranchDest = true;
 
+  bool IsUnconditionalBranch =
+  Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
   if (Parser.isParsingMasm()) {
-bool IsUnconditionalBranch =
-Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
 if (is64BitMode() && SM.getElementSize() > 0) {
   DefaultBaseReg = X86::RIP;
 }
@@ -2652,6 +2655,9 @@
 }
   }
 }
+  } else if (IsUnconditionalBranch) {
+if (PtrInOperand || SM.isBracketUsed())
+  MaybeDirectBranchDest = false;
   }
 
   if ((BaseReg || IndexReg || RegNo || DefaultBaseReg != X86::NoRegister))
Index: clang/test/CodeGen/ms-inline-asm-functions.c
===
--- clang/test/CodeGen/ms-inline-asm-functions.c
+++ clang/test/CodeGen/ms-inline-asm-functions.c
@@ -24,10 +24,9 @@
   __asm call kimport;
   // CHECK: calll   *({{.*}})
 
-  // Broken case: Call through a global function pointer.
+  // Call through a global function pointer.
   __asm call kptr;
-  // CHECK: calll   _kptr
-  // CHECK-FIXME: calll   *_kptr
+  // CHECK: calll   *_kptr
 }
 
 int bar(void) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 520158.
alvinhochun added a comment.

Apply suggestions: remove TODO, put negative tests in the same file as positive 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/test/MC/X86/intel-syntax-branch.s

Index: llvm/test/MC/X86/intel-syntax-branch.s
===
--- /dev/null
+++ llvm/test/MC/X86/intel-syntax-branch.s
@@ -0,0 +1,71 @@
+// RUN: llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-32,CHECK
+// RUN: llvm-mc -triple x86_64-unknown-unknown --defsym X64=1 -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-64,CHECK
+
+// RUN: not llvm-mc -triple i686-unknown-unknown --defsym ERR=1 -x86-asm-syntax=intel %s 2>&1 | FileCheck %s --check-prefixes=ERR-32
+
+t0:
+call direct_branch
+jmp direct_branch
+// CHECK-LABEL: t0:
+// CHECK-64: callq direct_branch
+// CHECK-32: calll direct_branch
+// CHECK:jmp direct_branch
+
+t1:
+call [fn_ref]
+jmp [fn_ref]
+// CHECK-LABEL: t1:
+// CHECK-64: callq *fn_ref
+// CHECK-64: jmpq *fn_ref
+// CHECK-32: calll *fn_ref
+// CHECK-32: jmpl *fn_ref
+
+.ifdef X64
+
+  t2:
+  call qword ptr [fn_ref]
+  jmp qword ptr [fn_ref]
+  // CHECK-64-LABEL: t2:
+  // CHECK-64: callq *fn_ref
+  // CHECK-64: jmpq *fn_ref
+
+  t3:
+  call qword ptr [rip + fn_ref]
+  jmp qword ptr [rip + fn_ref]
+  // CHECK-64-LABEL: t3:
+  // CHECK-64: callq *fn_ref(%rip)
+  // CHECK-64: jmpq *fn_ref(%rip)
+
+.else
+
+  t4:
+  call dword ptr [fn_ref]
+  jmp dword ptr [fn_ref]
+  // CHECK-32-LABEL: t4:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t5:
+  call dword ptr fn_ref
+  jmp dword ptr fn_ref
+  // CHECK-32-LABEL: t5:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t6:
+  call dword ptr [offset fn_ref]
+  jmp dword ptr [offset fn_ref]
+  // CHECK-32-LABEL: t6:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+.ifdef ERR
+
+  call offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+  jmp offset fn_ref
+  // ERR-32: {{.*}}.s:[[#@LINE-1]]:3: error: invalid operand for instruction
+
+.endif
+
+.endif
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -432,6 +432,7 @@
 InlineAsmIdentifierInfo Info;
 short BracCount = 0;
 bool MemExpr = false;
+bool BracketUsed = false;
 bool OffsetOperator = false;
 bool AttachToOperandIdx = false;
 bool IsPIC = false;
@@ -454,6 +455,7 @@
 void addImm(int64_t imm) { Imm += imm; }
 short getBracCount() const { return BracCount; }
 bool isMemExpr() const { return MemExpr; }
+bool isBracketUsed() const { return BracketUsed; }
 bool isOffsetOperator() const { return OffsetOperator; }
 SMLoc getOffsetLoc() const { return OffsetOperatorLoc; }
 unsigned getBaseReg() const { return BaseReg; }
@@ -954,6 +956,7 @@
 break;
   }
   MemExpr = true;
+  BracketUsed = true;
   BracCount++;
   return false;
 }
@@ -2630,9 +2633,9 @@
   unsigned DefaultBaseReg = X86::NoRegister;
   bool MaybeDirectBranchDest = true;
 
+  bool IsUnconditionalBranch =
+  Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
   if (Parser.isParsingMasm()) {
-bool IsUnconditionalBranch =
-Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
 if (is64BitMode() && SM.getElementSize() > 0) {
   DefaultBaseReg = X86::RIP;
 }
@@ -2654,6 +2657,9 @@
 }
   }
 }
+  } else if (IsUnconditionalBranch) {
+if (PtrInOperand || SM.isBracketUsed())
+  MaybeDirectBranchDest = false;
   }
 
   if ((BaseReg || IndexReg || RegNo || DefaultBaseReg != X86::NoRegister))
Index: clang/test/CodeGen/ms-inline-asm-functions.c
===
--- clang/test/CodeGen/ms-inline-asm-functions.c
+++ clang/test/CodeGen/ms-inline-asm-functions.c
@@ -24,10 +24,9 @@
   __asm call kimport;
   // CHECK: calll   *({{.*}})
 
-  // Broken case: Call through a global function pointer.
+  // Call through a global function pointer.
   __asm call kptr;
-  // CHECK: calll   _kptr
-  // CHECK-FIXME: calll   *_kptr
+  // CHECK: calll   *_kptr
 }
 
 int bar(void) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun marked an inline comment as not done.
alvinhochun added a comment.

In D149579#4320765 , @MaskRay wrote:

> OK. I  think after D149695  (landed), 
> D149920 , and this patch D149579 
> , these cases should be correct.

Thank you so much for looking into this!

I have one question in inline comment.




Comment at: llvm/test/MC/X86/intel-syntax-branch.s:61-67
+  // FIXME: MASM does not accept this syntax and GAS assembles this as a direct
+  //call/jump instead of indirect. Consider making this syntax an 
error?
+  call [offset fn_ref]
+  jmp [offset fn_ref]
+  // CHECK-32-LABEL: t7:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref

About the `call [offset fn_ref]` case, do you think we should reject it since 
MASM does not accept this syntax and GAS appears to behave oddly with it? The 
`ms-inline-asm-functions.c` test does generate `call [offset _kptr]` so this 
will also need to be changed.



Comment at: llvm/test/MC/X86/intel-syntax-branch.s:48
+  call dword ptr fn_ref
+  jmp dword ptr fn_ref
+  // CHECK-32-LABEL: t5:

MaskRay wrote:
> ICC and MSVC parse this differently.
> 
> Is this syntax valid?
MASM ml.exe assembles this as `jmp dword ptr [fn_ref]` in my test, so does GAS. 
I don't know how valid this syntax is though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579

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

[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 519869.
alvinhochun edited the summary of this revision.
alvinhochun added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rebase and add test cases with `offset` operator, and some TODOs/FIXMEs for 
comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579

Files:
  clang/test/CodeGen/ms-inline-asm-functions.c
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/test/MC/X86/intel-syntax-branch-fail.s
  llvm/test/MC/X86/intel-syntax-branch.s

Index: llvm/test/MC/X86/intel-syntax-branch.s
===
--- /dev/null
+++ llvm/test/MC/X86/intel-syntax-branch.s
@@ -0,0 +1,69 @@
+// RUN: llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-32,CHECK
+// RUN: llvm-mc -triple x86_64-unknown-unknown --defsym X64=1 -x86-asm-syntax=intel %s | FileCheck %s --check-prefixes=CHECK-64,CHECK
+
+t0:
+call direct_branch
+jmp direct_branch
+// CHECK-LABEL: t0:
+// CHECK-64: callq direct_branch
+// CHECK-32: calll direct_branch
+// CHECK:jmp direct_branch
+
+t1:
+call [fn_ref]
+jmp [fn_ref]
+// CHECK-LABEL: t1:
+// CHECK-64: callq *fn_ref
+// CHECK-64: jmpq *fn_ref
+// CHECK-32: calll *fn_ref
+// CHECK-32: jmpl *fn_ref
+
+.ifdef X64
+
+  t2:
+  call qword ptr [fn_ref]
+  jmp qword ptr [fn_ref]
+  // CHECK-64-LABEL: t2:
+  // CHECK-64: callq *fn_ref
+  // CHECK-64: jmpq *fn_ref
+
+  t3:
+  call qword ptr [rip + fn_ref]
+  jmp qword ptr [rip + fn_ref]
+  // CHECK-64-LABEL: t3:
+  // CHECK-64: callq *fn_ref(%rip)
+  // CHECK-64: jmpq *fn_ref(%rip)
+
+.else
+
+  t4:
+  call dword ptr [fn_ref]
+  jmp dword ptr [fn_ref]
+  // CHECK-32-LABEL: t4:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t5:
+  call dword ptr fn_ref
+  jmp dword ptr fn_ref
+  // CHECK-32-LABEL: t5:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t6:
+  call dword ptr [offset fn_ref]
+  jmp dword ptr [offset fn_ref]
+  // CHECK-32-LABEL: t6:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+  t7:
+  // FIXME: MASM does not accept this syntax and GAS assembles this as a direct
+  //call/jump instead of indirect. Consider making this syntax an error?
+  call [offset fn_ref]
+  jmp [offset fn_ref]
+  // CHECK-32-LABEL: t7:
+  // CHECK-32: calll *fn_ref
+  // CHECK-32: jmpl *fn_ref
+
+.endif
Index: llvm/test/MC/X86/intel-syntax-branch-fail.s
===
--- /dev/null
+++ llvm/test/MC/X86/intel-syntax-branch-fail.s
@@ -0,0 +1,12 @@
+// RUN: not llvm-mc -triple i686-unknown-unknown -x86-asm-syntax=intel %s 2>&1 | FileCheck %s
+
+// TODO: Consider making this syntax an error?
+// call [offset fn_ref]
+// // TODO-CHECK: {{.*}}intel-syntax-branch-fail.s:[[#@LINE+-1]]:1: error: `OFFSET` operator cannot be used in an unconditional branch
+// jmp [offset fn_ref]
+// // TODO-CHECK: {{.*}}intel-syntax-branch-fail.s:[[#@LINE+-1]]:1: error: `OFFSET` operator cannot be used in an unconditional branch
+
+call offset fn_ref
+// CHECK: {{.*}}intel-syntax-branch-fail.s:[[#@LINE+-1]]:1: error: invalid operand for instruction
+jmp offset fn_ref
+// CHECK: {{.*}}intel-syntax-branch-fail.s:[[#@LINE+-1]]:1: error: invalid operand for instruction
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -432,6 +432,7 @@
 InlineAsmIdentifierInfo Info;
 short BracCount = 0;
 bool MemExpr = false;
+bool BracketUsed = false;
 bool OffsetOperator = false;
 bool AttachToOperandIdx = false;
 bool IsPIC = false;
@@ -454,6 +455,7 @@
 void addImm(int64_t imm) { Imm += imm; }
 short getBracCount() const { return BracCount; }
 bool isMemExpr() const { return MemExpr; }
+bool isBracketUsed() const { return BracketUsed; }
 bool isOffsetOperator() const { return OffsetOperator; }
 SMLoc getOffsetLoc() const { return OffsetOperatorLoc; }
 unsigned getBaseReg() const { return BaseReg; }
@@ -954,6 +956,7 @@
 break;
   }
   MemExpr = true;
+  BracketUsed = true;
   BracCount++;
   return false;
 }
@@ -2630,9 +2633,9 @@
   unsigned DefaultBaseReg = X86::NoRegister;
   bool MaybeDirectBranchDest = true;
 
+  bool IsUnconditionalBranch =
+  Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
   if (Parser.isParsingMasm()) {
-bool IsUnconditionalBranch =
-Name.equals_insensitive("jmp") || Name.equals_insensitive("call");
 if (is64BitMode() && SM.getElementSize() > 0) {
   DefaultBaseReg = X86::RIP;
 }
@@ -2654,6 +2657,14 @@
 }
   }
 }
+  } else if (IsUnconditionalBranch) {
+// TODO: Consider making the `call [offset 

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG81358e9193a9: [clang][MinGW] Add asan DLL lib before other 
libs and objects (authored by alvinhochun).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-sanitizers.c


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,20 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// ASAN-ALL:"-lcomponent"
+// ASAN-ALL:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
+// ASAN-I686:   "--require-defined" "___asan_seh_interceptor"
+// ASAN-I686:   "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
 // ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a"
 // ASAN-X86_64: "--require-defined" "__asan_seh_interceptor"
-// ASAN-X86_64: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-X86_64: "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
 
 // RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=vptr
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -200,6 +200,16 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
   Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
+  // Add asan_dynamic as the first import lib before other libs. This allows
+  // asan to be initialized as early as possible to increase its 
instrumentation
+  // coverage to include other user DLLs which has not been built with asan.
+  if (Sanitize.needsAsanRt() && !Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+// MinGW always links against a shared MSVCRT.
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic", ToolChain::FT_Shared));
+  }
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_mdll)) {
   CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("dllcrt2.o")));


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,20 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" "{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address -lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 -DINPUT=%/t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added inline comments.



Comment at: clang/test/Driver/mingw-sanitizers.c:2
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s

mstorsjo wrote:
> You're using `%/t.a` here while the file you touch is `%t.a` - I don't think 
> I have seen `%/t` anywhere here before. I do see that lit seems to replace it 
> though... Is this an intentional thing (what's the difference to `%t` 
> though?) or is it a consistently copied typo?
Ah yes, that is intentional. It's documented on 
https://llvm.org/docs/CommandGuide/lit.html#substitutions to be "%t but \ is 
replaced by /". The reason for this is that, the previous Windows pre-merge 
check failed because when `clang -###` prints the command line it quotes and 
backslash-escape the arguments., FileCheck cannot match the INPUT path with 
double backslashes. Using forward slashes avoids this issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

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

[PATCH] D146908: [clang][MinGW] Add asan DLL lib before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 509030.
alvinhochun retitled this revision from "[clang][MinGW] Add asan link flags 
before other libs and objects" to "[clang][MinGW] Add asan DLL lib before other 
libs and objects".
alvinhochun added a comment.

Updated per comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-sanitizers.c


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,20 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// ASAN-ALL:"-lcomponent"
+// ASAN-ALL:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
+// ASAN-I686:   "--require-defined" "___asan_seh_interceptor"
+// ASAN-I686:   "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
 // ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a"
 // ASAN-X86_64: "--require-defined" "__asan_seh_interceptor"
-// ASAN-X86_64: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-X86_64: "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
 
 // RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=vptr
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -200,6 +200,16 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
   Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
+  // Add asan_dynamic as the first import lib before other libs. This allows
+  // asan to be initialized as early as possible to increase its 
instrumentation
+  // coverage to include other user DLLs which has not been built with asan.
+  if (Sanitize.needsAsanRt() && !Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+// MinGW always links against a shared MSVCRT.
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic", ToolChain::FT_Shared));
+  }
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_mdll)) {
   CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("dllcrt2.o")));


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,20 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" "{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: touch %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address -lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 -DINPUT=%/t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// 

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added inline comments.



Comment at: clang/test/Driver/mingw-sanitizers.c:1
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s

mstorsjo wrote:
> alvinhochun wrote:
> > mstorsjo wrote:
> > > Hm, what does this do - some sort of more portable version of just 
> > > touching a file to create it?
> > Yeah that's just creating the file (though it would also overwrite the 
> > existing content). I believe lit has `echo` built-in but `touch` may not be 
> > available on Windows so this seems like a good idea to me.
> Ah, I see. FWIW, the lit tests in general do assume a handful of unix 
> utilities - we seem to have unconditional uses of `touch` in various tests. 
> The lit tests check if these tools are available in path, and if they aren't, 
> it tries to locate an installation of Git for Windows (which contains what's 
> needed) and add that to the path internally: 
> https://github.com/llvm/llvm-project/blob/llvmorg-17-init/llvm/utils/lit/lit/llvm/config.py#L29-L35
I suppose that works. Would you prefer to use touch here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

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

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added a comment.

In D146908#4226926 , @mstorsjo wrote:

> I tested this, and this does fix the repro from the linked issue.

Thanks for testing!

> I wonder if we do want to move all of these before the app itself, or if it's 
> enough to just move the reference to `asan_dynamic-.dll.a` earlier, and 
> keep the rest as it was? Especially the whole-archive bit ends up linking a 
> bunch of things statically, ordered way before the app itself. On one hand, I 
> wonder if it would have subtle effects that we don't notice until way later, 
> and we should only change as little as we need (moving the import library 
> reference earlier), or if there are other potential benefits to moving the 
> wholearchive-linked bits earlier too? (I'm wondering about things like 
> constructor execution orders and such.)

That is a valid concern, especially if we want to backport this change to 16.x. 
I didn't think about this clearly before. With this in mind, I would change the 
patch to only add a duplicate of `asan_dynamic-.dll.a` early and keep the 
existing flags at their old position.

I can do some experiments with the new asan test setup I have and see if there 
are other benefits of moving the other flags. That will be for separate patches.




Comment at: clang/test/Driver/mingw-sanitizers.c:1
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s

mstorsjo wrote:
> Hm, what does this do - some sort of more portable version of just touching a 
> file to create it?
Yeah that's just creating the file (though it would also overwrite the existing 
content). I believe lit has `echo` built-in but `touch` may not be available on 
Windows so this seems like a good idea to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

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

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 508751.
alvinhochun added a comment.

Trying to fix the test...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-sanitizers.c


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,18 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
+// ASAN-I686:   "--require-defined" "___asan_seh_interceptor"
+// ASAN-I686:   "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
 // ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a"
 // ASAN-X86_64: "--require-defined" "__asan_seh_interceptor"
-// ASAN-X86_64: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-X86_64: "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-ALL:"-lcomponent"
+// ASAN-ALL:"[[INPUT]]"
 
 // RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=vptr
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -200,6 +200,29 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
   Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
+  // Add asan flags before other libs and objects. Making asan_dynamic the 
first
+  // import lib allows asan to be initialized as early as possible to increase
+  // its instrumentation coverage to include other user DLLs which has not been
+  // built with asan.
+  if (Sanitize.needsAsanRt() && !Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+// MinGW always links against a shared MSVCRT.
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic", ToolChain::FT_Shared));
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--require-defined");
+CmdArgs.push_back(TC.getArch() == llvm::Triple::x86
+  ? "___asan_seh_interceptor"
+  : "__asan_seh_interceptor");
+// Make sure the linker consider all object files from the dynamic
+// runtime thunk.
+CmdArgs.push_back("--whole-archive");
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--no-whole-archive");
+  }
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_mdll)) {
   CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("dllcrt2.o")));
@@ -292,24 +315,6 @@
   if (Args.hasArg(options::OPT_pthread))
 CmdArgs.push_back("-lpthread");
 
-  if (Sanitize.needsAsanRt()) {
-// MinGW always links against a shared MSVCRT.
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dynamic",
-ToolChain::FT_Shared));
-CmdArgs.push_back(
-TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
-CmdArgs.push_back("--require-defined");
-CmdArgs.push_back(TC.getArch() == llvm::Triple::x86
-  ? "___asan_seh_interceptor"
-  : "__asan_seh_interceptor");
-// Make sure the linker consider all object files from the dynamic
-// runtime thunk.
-CmdArgs.push_back("--whole-archive");
-CmdArgs.push_back(
-  

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
alvinhochun added reviewers: mstorsjo, MaskRay.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As stated in https://github.com/llvm/llvm-project/issues/61685, by
passing LLD the import lib of the asan DLL first, the asan DLL will be
listed as the first entry in the Import Directory Table, making it be
loaded first before other user DLLs. This allows asan to be initialized
as early as possible to increase its instrumentation coverage to include
other DLLs not built with asan.

This also avoids some false asan reports on `realloc` for memory
allocated during initialization of user DLLs being loaded earlier than
asan, because after this change they will be loaded later than asan.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146908

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-sanitizers.c


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,18 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
+// ASAN-I686:   "--require-defined" "___asan_seh_interceptor"
+// ASAN-I686:   "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
 // ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a"
 // ASAN-X86_64: "--require-defined" "__asan_seh_interceptor"
-// ASAN-X86_64: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-X86_64: "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-ALL:"-lcomponent"
+// ASAN-ALL:"[[INPUT]]"
 
 // RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=vptr
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -200,6 +200,29 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
   Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
+  // Add asan flags before other libs and objects. Making asan_dynamic the 
first
+  // import lib allows asan to be initialized as early as possible to increase
+  // its instrumentation coverage to include other user DLLs which has not been
+  // built with asan.
+  if (Sanitize.needsAsanRt() && !Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+// MinGW always links against a shared MSVCRT.
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic", ToolChain::FT_Shared));
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--require-defined");
+CmdArgs.push_back(TC.getArch() == llvm::Triple::x86
+  ? "___asan_seh_interceptor"
+  : "__asan_seh_interceptor");
+// Make sure the linker consider all object files from the dynamic
+// runtime thunk.
+CmdArgs.push_back("--whole-archive");
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--no-whole-archive");
+  }
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_mdll)) {
   CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("dllcrt2.o")));
@@ -292,24 +315,6 @@
   if (Args.hasArg(options::OPT_pthread))
 CmdArgs.push_back("-lpthread");
 
-  if (Sanitize.needsAsanRt()) {
-// MinGW always links against a shared MSVCRT.
-

[PATCH] D142346: [docs] Add release notes for news in 16.x done by me, or otherwise relating to MinGW targets

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun accepted this revision.
alvinhochun added a comment.

Thanks, LGTM for the parts I touched and also the rest of the stuff (but I 
didn't proofread carefully).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142346

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

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun accepted this revision.
alvinhochun added a comment.

Also lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141206

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

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added inline comments.



Comment at: clang/test/Driver/mingw-sysroot.cpp:25-38
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %clang -target 
x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 
2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE:[^"]+]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}c++"
 // CHECK_TESTROOT_GCC-SAME: {{^}} "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}c++{{/|}}x86_64-w64-mingw32"
 // CHECK_TESTROOT_GCC-SAME: {{^}} "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}c++{{/|}}backward"
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}g++-v10.2-posix"
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}g++-v10.2"
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}g++-v10"

mstorsjo wrote:
> alvinhochun wrote:
> > I think this only verifies that the include path is not present after the 
> > last `CHECK_TESTROOT_GCC` match. The [[ 
> > https://llvm.org/docs/CommandGuide/FileCheck.html#cmdoption-filecheck-implicit-check-not
> >  | --implicit-check-not ]] option should be used to check that the pattern 
> > is not present anywhere in the output.
> Right - that's true (although this is the spot where the entry did appear 
> before - I checked that the test did fail before applying the functional 
> change).
> 
> I tried adding `--implicit-check-not` here, but the quoting/escaping of the 
> regexes seems tricky - even getting it to recognize the literal quotes 
> surrounding the string.
> 
> I tried with 
> `--implicit-check-not="\"{{.*}}/testroot-gcc{{/|}}x86_64-w64-mingw32{{/|}}include\""`,
>  but that's not working (i.e. it's not detecting anything even when I remove 
> the functional change of the patch). With a trivial reduction of it, into 
> `--implicit-check-not="testroot-gcc/x86_64-w64-mingw32/include"`, it's 
> detecting things as expected (but that quote gets handled by the shell, so 
> this would match any substring, such as `.../include/subdir` too. But even 
> `--implicit-check-not="testroot-gcc/x86_64-w64-mingw32/include\""` does not 
> match correctly.
> 
> The documentation says that the `--implicit-check-not` option does take a 
> pattern, but it doesn't really say what's allowed in that pattern - 
> apparently not the full syntax used in normal `CHECK` lines at least...
Maybe it's a problem with escaping those backslashes? I think the command line 
is interpreted by lit which has its own escaping rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141206

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

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added inline comments.



Comment at: clang/test/Driver/mingw-sysroot.cpp:25-38
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %clang -target 
x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 
2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE:[^"]+]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}c++"
 // CHECK_TESTROOT_GCC-SAME: {{^}} "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}c++{{/|}}x86_64-w64-mingw32"
 // CHECK_TESTROOT_GCC-SAME: {{^}} "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}c++{{/|}}backward"
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}g++-v10.2-posix"
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}g++-v10.2"
 // CHECK_TESTROOT_GCC: "-internal-isystem" 
"[[BASE]]/testroot-gcc{{/|}}lib{{/|}}gcc{{/|}}x86_64-w64-mingw32{{/|}}10.2-posix{{/|}}include{{/|}}g++-v10"

I think this only verifies that the include path is not present after the last 
`CHECK_TESTROOT_GCC` match. The [[ 
https://llvm.org/docs/CommandGuide/FileCheck.html#cmdoption-filecheck-implicit-check-not
 | --implicit-check-not ]] option should be used to check that the pattern is 
not present anywhere in the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141206

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

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added a comment.

The idea sounds reasonable. I don't know mingw-w64 toolchains well enough, but 
I'll try:

How does it interact with the following conditions (lines 469-475)? They look 
like they may be looking for a mingw-w64 sysroot, which may be ignored by the 
new check.

  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot(
   getDriver(), LiteralTriple, getTriple(), SubdirName))
Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
  else if (llvm::ErrorOr GPPName =
   findGcc(LiteralTriple, getTriple()))
Base = std::string(llvm::sys::path::parent_path(
llvm::sys::path::parent_path(GPPName.get(;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141206

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

[PATCH] D135027: [Clang][MinGW][cygwin] Fix __declspec with -fdeclspec enabled

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
alvinhochun added reviewers: rnk, mstorsjo.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/49958


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135027

Files:
  clang/lib/Basic/Targets.cpp
  clang/test/Preprocessor/init-x86.c
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1368,9 +1368,12 @@
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-sie-ps5 < 
/dev/null | FileCheck --match-full-lines --check-prefix PS4-CXX %s
 // PS4-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 32UL
 //
-// RUN: %clang_cc1 -E -dM -triple=x86_64-pc-mingw32 < /dev/null | FileCheck 
-match-full-lines -check-prefix X86-64-DECLSPEC %s
-// RUN: %clang_cc1 -E -dM -fms-extensions -triple=x86_64-unknown-mingw32 < 
/dev/null | FileCheck -match-full-lines -check-prefix X86-64-DECLSPEC %s
-// X86-64-DECLSPEC: #define __declspec{{.*}}
+// RUN: %clang_cc1 -E -dM -triple=x86_64-pc-mingw32 < /dev/null | FileCheck 
-match-full-lines -check-prefix X86-64-DECLSPEC-GNU %s
+// X86-64-DECLSPEC-GNU: #define __declspec{{.*}} __attribute__{{.*}}
+//
+// RUN: %clang_cc1 -E -dM -fms-extensions -triple=x86_64-unknown-mingw32 < 
/dev/null | FileCheck -match-full-lines -check-prefix X86-64-DECLSPEC-MS %s
+// RUN: %clang_cc1 -E -dM -fdeclspec -triple=x86_64-unknown-mingw32 < 
/dev/null | FileCheck -match-full-lines -check-prefix X86-64-DECLSPEC-MS %s
+// X86-64-DECLSPEC-MS: #define __declspec{{.*}} __declspec{{.*}}
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=sparc64-none-none < /dev/null 
| FileCheck -match-full-lines -check-prefix SPARCV9 %s
 // SPARCV9:#define __BIGGEST_ALIGNMENT__ 16
Index: clang/test/Preprocessor/init-x86.c
===
--- clang/test/Preprocessor/init-x86.c
+++ clang/test/Preprocessor/init-x86.c
@@ -587,11 +587,15 @@
 // I386-NETBSD:#define __i386__ 1
 // I386-NETBSD:#define i386 1
 
-// RUN: %clang_cc1 -E -dM -triple=i686-pc-mingw32 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-DECLSPEC %s
-// RUN: %clang_cc1 -E -dM -fms-extensions -triple=i686-pc-mingw32 < /dev/null 
| FileCheck -match-full-lines -check-prefix I386-DECLSPEC %s
-// RUN: %clang_cc1 -E -dM -triple=i686-unknown-cygwin < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-DECLSPEC %s
-// RUN: %clang_cc1 -E -dM -fms-extensions -triple=i686-unknown-cygwin < 
/dev/null | FileCheck -match-full-lines -check-prefix I386-DECLSPEC %s
-// I386-DECLSPEC: #define __declspec{{.*}}
+// RUN: %clang_cc1 -E -dM -triple=i686-pc-mingw32 < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-DECLSPEC-GNU %s
+// RUN: %clang_cc1 -E -dM -triple=i686-unknown-cygwin < /dev/null | FileCheck 
-match-full-lines -check-prefix I386-DECLSPEC-GNU %s
+// I386-DECLSPEC-GNU: #define __declspec{{.*}} __attribute__{{.*}}
+
+// RUN: %clang_cc1 -E -dM -fms-extensions -triple=i686-pc-mingw32 < /dev/null 
| FileCheck -match-full-lines -check-prefix I386-DECLSPEC-MS %s
+// RUN: %clang_cc1 -E -dM -fdeclspec -triple=i686-pc-mingw32 < /dev/null | 
FileCheck -match-full-lines -check-prefix I386-DECLSPEC-MS %s
+// RUN: %clang_cc1 -E -dM -fms-extensions -triple=i686-unknown-cygwin < 
/dev/null | FileCheck -match-full-lines -check-prefix I386-DECLSPEC-MS %s
+// RUN: %clang_cc1 -E -dM -fdeclspec -triple=i686-unknown-cygwin < /dev/null | 
FileCheck -match-full-lines -check-prefix I386-DECLSPEC-MS %s
+// I386-DECLSPEC-MS: #define __declspec{{.*}} __declspec{{.*}}
 
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=x86_64-none-none < /dev/null | FileCheck -match-full-lines 
-check-prefix X86_64 %s
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -81,9 +81,10 @@
 
 void addCygMingDefines(const LangOptions , MacroBuilder ) {
   // Mingw and cygwin define __declspec(a) to __attribute__((a)).  Clang
-  // supports __declspec natively under -fms-extensions, but we define a no-op
-  // __declspec macro anyway for pre-processor compatibility.
-  if (Opts.MicrosoftExt)
+  // supports __declspec natively under -fdeclspec (also enabled with
+  // -fms-extensions), but we define a no-op __declspec macro anyway for
+  // pre-processor compatibility.
+  if (Opts.DeclSpecKeyword)
 Builder.defineMacro("__declspec", "__declspec");
   else
 Builder.defineMacro("__declspec(a)", "__attribute__((a))");


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1368,9 +1368,12 @@
 // RUN: %clang_cc1 

[PATCH] D133773: [clang] fix linker executable path in test

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added a comment.

The literal dots have not been escaped as `\.`, but the current change works 
anyway and the chance of false positives here should be very low. I guess this 
is acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133773

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

[PATCH] D133773: [clang] fix linker executable path in test

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added a comment.

In D133773#3786602 , @mstorsjo wrote:

> In D133773#3786484 , @alvinhochun 
> wrote:
>
>> Ah sorry about that, I didn't realize the command includes `.exe` on 
>> Windows. (For the record, `ld"` matches both `ld` and `ld.lld`.)
>
> No, iirc these quotes are checked as literal part of the matched string here; 
> otherwise the pattern `ld"` would match `ld.exe"` too. So to match any linker 
> name, I'd write `ld{{.*}}"` or maybe something like `ld{{[\.a-z]*}}"`, modulo 
> regex syntax.

Oops, I meant `ld"` and `ld.lld"`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133773

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

[PATCH] D133773: [clang] fix linker executable path in test

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun added a comment.

Ah sorry about that, I didn't realize the command includes `.exe` on Windows. 
(For the record, `ld"` matches both `ld` and `ld.lld`.)




Comment at: clang/test/Driver/mingw-cfguard.c:6
 // NO_CF-NOT: "-cfguard-no-checks"
-// NO_CF-NEXT: ld"
+// NO_CF-NEXT: ld{{(.exe)?}}"
 // NO_CF-NOT: "--guard-cf"

Literal dot needs to be escaped as `\.` in regex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133773

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

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 458392.
alvinhochun added a comment.

Fixed `--target=` option for test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132810

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-cfguard.c

Index: clang/test/Driver/mingw-cfguard.c
===
--- /dev/null
+++ clang/test/Driver/mingw-cfguard.c
@@ -0,0 +1,28 @@
+// RUN: %clang --target=x86_64-w64-windows-gnu -### %s 2>&1 | FileCheck -check-prefixes=NO_CF,DEFAULT %s
+// RUN: %clang --target=x86_64-w64-windows-gnu -### %s -mguard=none 2>&1 | FileCheck -check-prefixes=NO_CF,GUARD_NONE %s
+// NO_CF: "-cc1"
+// NO_CF-NOT: "-cfguard"
+// NO_CF-NOT: "-cfguard-no-checks"
+// NO_CF-NEXT: ld"
+// NO_CF-NOT: "--guard-cf"
+// DEFAULT-NOT: "--no-guard-cf"
+// GUARD_NONE-SAME: "--no-guard-cf"
+
+// RUN: %clang --target=x86_64-w64-windows-gnu -### %s -mguard=cf 2>&1 | FileCheck -check-prefix=GUARD_CF %s
+// GUARD_CF: "-cc1"
+// GUARD_CF-SAME: "-cfguard"
+// GUARD_CF-NEXT: ld"
+// GUARD_CF-SAME: "--guard-cf"
+// GUARD_CF-NOT: "--no-guard-cf"
+
+// RUN: %clang --target=x86_64-w64-windows-gnu -### %s -mguard=cf-nochecks 2>&1 | FileCheck -check-prefix=GUARD_NOCHECKS %s
+// GUARD_NOCHECKS: "-cc1"
+// GUARD_NOCHECKS-NOT: "-cfguard"
+// GUARD_NOCHECKS-SAME: "-cfguard-no-checks"
+// GUARD_NOCHECKS-NOT: "-cfguard"
+// GUARD_NOCHECKS-NEXT: ld"
+// GUARD_NOCHECKS-SAME: "--guard-cf"
+// GUARD_NOCHECKS-NOT: "--no-guard-cf"
+
+// RUN: %clang --target=x86_64-w64-windows-gnu -### %s -mguard=xxx 2>&1 | FileCheck -check-prefix=GUARD_UNKNOWN %s
+// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '--mguard='
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -79,6 +79,10 @@
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -169,6 +169,17 @@
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
 CmdArgs.push_back("--no-demangle");
 
+  if (Arg *A = Args.getLastArg(options::OPT_mguard_EQ)) {
+StringRef GuardArgs = A->getValue();
+if (GuardArgs == "none")
+  CmdArgs.push_back("--no-guard-cf");
+else if (GuardArgs == "cf" || GuardArgs == "cf-nochecks")
+  CmdArgs.push_back("--guard-cf");
+else
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << GuardArgs;
+  }
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name
@@ -607,6 +618,26 @@
   addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
+void toolchains::MinGW::addClangTargetOptions(
+const llvm::opt::ArgList , llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (Arg *A = DriverArgs.getLastArg(options::OPT_mguard_EQ)) {
+StringRef GuardArgs = A->getValue();
+if (GuardArgs == "none") {
+  // Do nothing.
+} else if (GuardArgs == "cf") {
+  // Emit CFG instrumentation and the table of address-taken functions.
+  CC1Args.push_back("-cfguard");
+} else if (GuardArgs == "cf-nochecks") {
+  // Emit only the table of address-taken functions.
+  CC1Args.push_back("-cfguard-no-checks");
+} else {
+  getDriver().Diag(diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << GuardArgs;
+}
+  }
+}
+
 void toolchains::MinGW::AddClangCXXStdlibIncludeArgs(
 const ArgList , ArgStringList ) const {
   if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3360,6 +3360,9 @@
 def mdll : Joined<["-"], "mdll">, Group, Flags<[NoXarchOption]>;
 def municode : Joined<["-"], "municode">, Group, Flags<[NoXarchOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, Flags<[NoXarchOption]>;
+def mguard_EQ : Joined<["-"], "mguard=">, Group, Flags<[NoXarchOption]>,
+  HelpText<"Enable or disable Control Flow Guard checks and guard tables 

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 457523.
alvinhochun added a comment.

Applied suggestions. Thanks for the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132810

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-cfguard.c

Index: clang/test/Driver/mingw-cfguard.c
===
--- /dev/null
+++ clang/test/Driver/mingw-cfguard.c
@@ -0,0 +1,28 @@
+// RUN: %clang -target=x86_64-w64-windows-gnu -### %s 2>&1 | FileCheck -check-prefixes=NO_CF,DEFAULT %s
+// RUN: %clang -target=x86_64-w64-windows-gnu -### %s -mguard=none 2>&1 | FileCheck -check-prefixes=NO_CF,GUARD_NONE %s
+// NO_CF: "-cc1"
+// NO_CF-NOT: "-cfguard"
+// NO_CF-NOT: "-cfguard-no-checks"
+// NO_CF-NEXT: ld"
+// NO_CF-NOT: "--guard-cf"
+// DEFAULT-NOT: "--no-guard-cf"
+// GUARD_NONE-SAME: "--no-guard-cf"
+
+// RUN: %clang -target=x86_64-w64-windows-gnu -### %s -mguard=cf 2>&1 | FileCheck -check-prefix=GUARD_CF %s
+// GUARD_CF: "-cc1"
+// GUARD_CF-SAME: "-cfguard"
+// GUARD_CF-NEXT: ld"
+// GUARD_CF-SAME: "--guard-cf"
+// GUARD_CF-NOT: "--no-guard-cf"
+
+// RUN: %clang -target=x86_64-w64-windows-gnu -### %s -mguard=cf-nochecks 2>&1 | FileCheck -check-prefix=GUARD_NOCHECKS %s
+// GUARD_NOCHECKS: "-cc1"
+// GUARD_NOCHECKS-NOT: "-cfguard"
+// GUARD_NOCHECKS-SAME: "-cfguard-no-checks"
+// GUARD_NOCHECKS-NOT: "-cfguard"
+// GUARD_NOCHECKS-NEXT: ld"
+// GUARD_NOCHECKS-SAME: "--guard-cf"
+// GUARD_NOCHECKS-NOT: "--no-guard-cf"
+
+// RUN: %clang -target=x86_64-w64-windows-gnu -### %s -mguard=xxx 2>&1 | FileCheck -check-prefix=GUARD_UNKNOWN %s
+// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '--mguard='
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -79,6 +79,10 @@
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -169,6 +169,17 @@
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
 CmdArgs.push_back("--no-demangle");
 
+  if (Arg *A = Args.getLastArg(options::OPT_mguard_EQ)) {
+StringRef GuardArgs = A->getValue();
+if (GuardArgs == "none")
+  CmdArgs.push_back("--no-guard-cf");
+else if (GuardArgs == "cf" || GuardArgs == "cf-nochecks")
+  CmdArgs.push_back("--guard-cf");
+else
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << GuardArgs;
+  }
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name
@@ -607,6 +618,26 @@
   addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
+void toolchains::MinGW::addClangTargetOptions(
+const llvm::opt::ArgList , llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (Arg *A = DriverArgs.getLastArg(options::OPT_mguard_EQ)) {
+StringRef GuardArgs = A->getValue();
+if (GuardArgs == "none") {
+  // Do nothing.
+} else if (GuardArgs == "cf") {
+  // Emit CFG instrumentation and the table of address-taken functions.
+  CC1Args.push_back("-cfguard");
+} else if (GuardArgs == "cf-nochecks") {
+  // Emit only the table of address-taken functions.
+  CC1Args.push_back("-cfguard-no-checks");
+} else {
+  getDriver().Diag(diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << GuardArgs;
+}
+  }
+}
+
 void toolchains::MinGW::AddClangCXXStdlibIncludeArgs(
 const ArgList , ArgStringList ) const {
   if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3360,6 +3360,9 @@
 def mdll : Joined<["-"], "mdll">, Group, Flags<[NoXarchOption]>;
 def municode : Joined<["-"], "municode">, Group, Flags<[NoXarchOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, Flags<[NoXarchOption]>;
+def mguard_EQ : Joined<["-"], "mguard=">, Group, Flags<[NoXarchOption]>,
+  HelpText<"Enable or disable Control Flow Guard checks and guard tables 

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
Herald added a subscriber: mstorsjo.
Herald added a project: All.
alvinhochun published this revision for review.
alvinhochun added reviewers: rnk, ajpaverd, mstorsjo, aaron.ballman.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This option can be used to enable Control Flow Guard checks and
generation of address-taken function table. They are equivalent to
`/guard:cf` and `/guard:cf,nochecks` in clang-cl. Passing this flag to
the Clang driver will also pass `--guard-cf` to the MinGW linker.

This feature is disabled by default. The option `-mguard=none` is also
available to explicitly disable this feature.

Depends on D132808 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132810

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-cfguard.c

Index: clang/test/Driver/mingw-cfguard.c
===
--- /dev/null
+++ clang/test/Driver/mingw-cfguard.c
@@ -0,0 +1,34 @@
+// RUN: %clang -v -target x86_64-w64-windows-gnu -### %s 2>&1 | FileCheck -check-prefixes=NO_CF,DEFAULT %s
+// RUN: %clang -v -target x86_64-w64-windows-gnu -### %s -mguard=none 2>&1 | FileCheck -check-prefixes=NO_CF,GUARD_NONE %s
+// NO_CF: "-cc1"
+// NO_CF-NOT: "-cfguard"
+// NO_CF-NOT: "-cfguard-no-checks"
+// NO_CF-SAME: {{$}}
+// NO_CF-NEXT: ld"
+// NO_CF-NOT: "--guard-cf"
+// DEFAULT-NOT: "--no-guard-cf"
+// GUARD_NONE-SAME: "--no-guard-cf"
+// NO_CF-SAME: {{$}}
+
+// RUN: %clang -v -target x86_64-w64-windows-gnu -### %s -mguard=cf 2>&1 | FileCheck -check-prefix=GUARD_CF %s
+// GUARD_CF: "-cc1"
+// GUARD_CF-SAME: "-cfguard"
+// GUARD_CF-SAME: {{$}}
+// GUARD_CF-NEXT: ld"
+// GUARD_CF-SAME: "--guard-cf"
+// GUARD_CF-NOT: "--no-guard-cf"
+// GUARD_CF-SAME: {{$}}
+
+// RUN: %clang -v -target x86_64-w64-windows-gnu -### %s -mguard=cf-nochecks 2>&1 | FileCheck -check-prefix=GUARD_NOCHECKS %s
+// GUARD_NOCHECKS: "-cc1"
+// GUARD_NOCHECKS-NOT: "-cfguard"
+// GUARD_NOCHECKS-SAME: "-cfguard-no-checks"
+// GUARD_NOCHECKS-NOT: "-cfguard"
+// GUARD_NOCHECKS-SAME: {{$}}
+// GUARD_NOCHECKS-NEXT: ld"
+// GUARD_NOCHECKS-SAME: "--guard-cf"
+// GUARD_NOCHECKS-NOT: "--no-guard-cf"
+// GUARD_NOCHECKS-SAME: {{$}}
+
+// RUN: %clang -v -target x86_64-w64-windows-gnu -### %s -mguard=xxx 2>&1 | FileCheck -check-prefix=GUARD_UNKNOWN %s
+// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '--mguard='
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -79,6 +79,10 @@
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -169,6 +169,18 @@
   if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
 CmdArgs.push_back("--no-demangle");
 
+  if (Arg *A = Args.getLastArg(options::OPT_mguard_EQ)) {
+StringRef GuardArgs = A->getValue();
+if (GuardArgs.equals_insensitive("none"))
+  CmdArgs.push_back("--no-guard-cf");
+else if (GuardArgs.equals_insensitive("cf") ||
+ GuardArgs.equals_insensitive("cf-nochecks"))
+  CmdArgs.push_back("--guard-cf");
+else
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getSpelling() << GuardArgs;
+  }
+
   CmdArgs.push_back("-o");
   const char *OutputFile = Output.getFilename();
   // GCC implicitly adds an .exe extension if it is given an output file name
@@ -607,6 +619,26 @@
   addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
+void toolchains::MinGW::addClangTargetOptions(
+const llvm::opt::ArgList , llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (Arg *A = DriverArgs.getLastArg(options::OPT_mguard_EQ)) {
+StringRef GuardArgs = A->getValue();
+if (GuardArgs.equals_insensitive("none")) {
+  // Do nothing.
+} else if (GuardArgs.equals_insensitive("cf")) {
+  // Emit CFG instrumentation and the table of address-taken functions.
+  CC1Args.push_back("-cfguard");
+} else if (GuardArgs.equals_insensitive("cf-nochecks")) {
+  // Emit only the table of address-taken functions.
+  CC1Args.push_back("-cfguard-no-checks");
+} else {
+  

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 455820.
alvinhochun added a comment.

Applied suggestions and added test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132661

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-guard_nocf.c


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 
-fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // 
expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // 
expected-warning {{unknown attribute 'guard' ignored}} 
+
+#endif
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win32"];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to 
``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions 
when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // expected-warning {{unknown attribute 'guard' ignored}} 
+
+#endif
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win32"];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = 

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
alvinhochun added reviewers: rnk, ajpaverd, mstorsjo, aaron.ballman.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Control Flow Guard is only supported on Windows target, therefore there
is no point to make it an accepted attribute for other targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132661

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetHasCFGuard : TargetSpec {
+  let CustomCode = [{ Target.getTriple().isOSWindows() }];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to 
``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions 
when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetHasCFGuard : TargetSpec {
+  let CustomCode = [{ Target.getTriple().isOSWindows() }];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to ``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 454798.
alvinhochun marked an inline comment as done.
alvinhochun added a comment.

Rebased and added a release note entry. @mstorsjo would you mind landing this 
for me? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132302

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-guard_nocf.c

Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,10 +1,20 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
 
+// Function definition using GNU-style attribute without relying on the
+// __declspec define for mingw.
+__attribute__((guard(nocf))) void testGNUStyleGuardNoCF(void) { // no warning
+}
+
 // Can not be used on variable, parameter, or function pointer declarations.
 int __declspec(guard(nocf)) i;  // expected-warning {{'guard' attribute only applies to functions}}
 void testGuardNoCFFuncParam(double __declspec(guard(nocf)) i) {}// expected-warning {{'guard' attribute only applies to functions}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -26,6 +26,7 @@
 // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: CFGuard (SubjectMatchRule_function)
 // CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
 // CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===
--- clang/test/CodeGenCXX/guard_nocf.cpp
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func();
 void (*func_ptr)() = _func;
@@ -10,6 +14,23 @@
 // CHECK-LABEL: nocf0
 // CHECK: call{{.*}}[[NOCF:#[0-9]+]]
 
+// Explicitly test using the GNU-style attribute without relying on the
+// __declspec define for mingw.
+// The "guard_nocf" attribute must be added.
+__attribute__((guard(nocf))) void nocf0_gnu_style() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_gnu_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// Test using the c++-style attribute.
+// The "guard_nocf" attribute must be added.
+[[clang::guard(nocf)]] void nocf0_cxx_style() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_cxx_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
 // The "guard_nocf" attribute must *not* be added.
 void cf0() {
   (*func_ptr)();
Index: clang/test/CodeGen/guard_nocf.c
===
--- clang/test/CodeGen/guard_nocf.c
+++ clang/test/CodeGen/guard_nocf.c
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -emit-llvm -O2 -o - %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func(void);
 void (*func_ptr)(void) = _func;
@@ -10,6 +14,15 @@
 // CHECK-LABEL: nocf0
 // CHECK: call{{.*}}[[NOCF:#[0-9]+]]
 
+// Explicitly test using the GNU-style attribute without relying on the
+// __declspec define for mingw.
+// The "guard_nocf" attribute must be added.
+__attribute__((guard(nocf))) void nocf0_gnu_style(void) {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_gnu_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
 // 

[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 454559.
alvinhochun edited the summary of this revision.
alvinhochun added a comment.

Applied suggestions from Aaron. Thanks for the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132302

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-guard_nocf.c

Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,10 +1,20 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
 
+// Function definition using GNU-style attribute without relying on the
+// __declspec define for mingw.
+__attribute__((guard(nocf))) void testGNUStyleGuardNoCF(void) { // no warning
+}
+
 // Can not be used on variable, parameter, or function pointer declarations.
 int __declspec(guard(nocf)) i;  // expected-warning {{'guard' attribute only applies to functions}}
 void testGuardNoCFFuncParam(double __declspec(guard(nocf)) i) {}// expected-warning {{'guard' attribute only applies to functions}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -26,6 +26,7 @@
 // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: CFGuard (SubjectMatchRule_function)
 // CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
 // CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===
--- clang/test/CodeGenCXX/guard_nocf.cpp
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func();
 void (*func_ptr)() = _func;
@@ -10,6 +14,23 @@
 // CHECK-LABEL: nocf0
 // CHECK: call{{.*}}[[NOCF:#[0-9]+]]
 
+// Explicitly test using the GNU-style attribute without relying on the
+// __declspec define for mingw.
+// The "guard_nocf" attribute must be added.
+__attribute__((guard(nocf))) void nocf0_gnu_style() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_gnu_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// Test using the c++-style attribute.
+// The "guard_nocf" attribute must be added.
+[[clang::guard(nocf)]] void nocf0_cxx_style() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_cxx_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
 // The "guard_nocf" attribute must *not* be added.
 void cf0() {
   (*func_ptr)();
Index: clang/test/CodeGen/guard_nocf.c
===
--- clang/test/CodeGen/guard_nocf.c
+++ clang/test/CodeGen/guard_nocf.c
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -emit-llvm -O2 -o - %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func(void);
 void (*func_ptr)(void) = _func;
@@ -10,6 +14,15 @@
 // CHECK-LABEL: nocf0
 // CHECK: call{{.*}}[[NOCF:#[0-9]+]]
 
+// Explicitly test using the GNU-style attribute without relying on the
+// __declspec define for mingw.
+// The "guard_nocf" attribute must be added.
+__attribute__((guard(nocf))) void nocf0_gnu_style(void) {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_gnu_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
 // The "guard_nocf" attribute must *not* be added.
 void cf0(void) {
   

[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 454280.
alvinhochun added a comment.

Specify x86_64-w64-windows-gnu in tests instead of using %itanium_abi_triple 
because it doesn't work on Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132302

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-guard_nocf.c


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,5 +1,10 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify 
-fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 
-fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -26,6 +26,7 @@
 // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: CFGuard (SubjectMatchRule_function)
 // CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
 // CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===
--- clang/test/CodeGenCXX/guard_nocf.cpp
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 
-emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -std=c++11 -emit-llvm -O2 -o 
- %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func();
 void (*func_ptr)() = _func;
Index: clang/test/CodeGen/guard_nocf.c
===
--- clang/test/CodeGen/guard_nocf.c
+++ clang/test/CodeGen/guard_nocf.c
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -emit-llvm -O2 -o - %s | 
FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func(void);
 void (*func_ptr)(void) = _func;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3493,7 +3493,7 @@
 def CFGuard : InheritableAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
-  let Spellings = [Declspec<"guard">];
+  let Spellings = [Declspec<"guard">, GCC<"guard">];
   let Subjects = SubjectList<[Function]>;
   let Args = [EnumArgument<"Guard", "GuardArg", ["nocf"], ["nocf"]>];
   let Documentation = [CFGuardDocs];


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,5 +1,10 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ 

[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
Herald added subscribers: pengfei, mstorsjo.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
alvinhochun added reviewers: ajpaverd, rnk.
alvinhochun updated this revision to Diff 454274.
alvinhochun edited the summary of this revision.
alvinhochun added a comment.
Herald added a subscriber: jdoerfert.
alvinhochun updated this revision to Diff 454277.
alvinhochun published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update patch


alvinhochun added a comment.

Revert llvm test changes


To support using Control Flow Guard with mingw-w64, Clang needs to
accept `__declspec(guard(nocf))` also for the GNU target. Since mingw
has `#define __declspec(a) __attribute__((a))` as built-in, the simplest
solution is to accept `__attribute((guard(nocf)))` to be compatible with
MSVC and Clang's msvc target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132302

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-guard_nocf.c


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,5 +1,10 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify 
-fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 
-fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -verify -std=c++11 
-fsyntax-only -x c++ %s
+
+// The %itanium_abi_triple version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -26,6 +26,7 @@
 // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: CFGuard (SubjectMatchRule_function)
 // CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
 // CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===
--- clang/test/CodeGenCXX/guard_nocf.cpp
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 
-emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -emit-llvm -O2 -o - 
%s | FileCheck %s
+
+// The %itanium_abi_triple version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func();
 void (*func_ptr)() = _func;
Index: clang/test/CodeGen/guard_nocf.c
===
--- clang/test/CodeGen/guard_nocf.c
+++ clang/test/CodeGen/guard_nocf.c
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -O2 -o - %s | 
FileCheck %s
+
+// The %itanium_abi_triple version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func(void);
 void (*func_ptr)(void) = _func;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3493,7 +3493,7 @@
 def CFGuard : InheritableAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
-  let Spellings = [Declspec<"guard">];
+  let Spellings = [Declspec<"guard">, GCC<"guard">];
   let Subjects = SubjectList<[Function]>;
   let Args = [EnumArgument<"Guard", "GuardArg", ["nocf"], ["nocf"]>];
   let Documentation = [CFGuardDocs];


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,5 +1,10 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple