[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-16 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1bd5ea968e92: [ARM] Mitigate the cve-2021-35465 security 
vulnurability. (authored by labrinea).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109157

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-cmse-cve-2021-35465.c
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
  llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir

Index: llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
===
--- llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
+++ llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
@@ -1,4 +1,4 @@
-# RUN: llc -mtriple=thumbv8m.main -mcpu=cortex-m33 --float-abi=hard --run-pass=arm-pseudo %s -o - | \
+# RUN: llc -mtriple=thumbv8m.main -mcpu=cortex-m33 -mattr=-fix-cmse-cve-2021-35465 --float-abi=hard --run-pass=arm-pseudo %s -o - | \
 # RUN: FileCheck %s
 --- |
   ; ModuleID = 'cmse-vlldm-no-reorder.ll'
@@ -109,4 +109,4 @@
 # CHECK-NEXT:  $s0 = VMOVSR $r12, 14 /* CC::al */, $noreg
 # CHECK-NEXT:  $sp = tADDspi $sp, 34, 14 /* CC::al */, $noreg
 # CHECK-NEXT:  $sp = t2LDMIA_UPD $sp, 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $r7, def $r8, def $r9, def $r10, def $r11
- 
\ No newline at end of file
+ 
Index: llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
@@ -0,0 +1,119 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -verify-machineinstrs \
+; RUN:   -mattr=+fp-armv8d16sp,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m33 -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m35p -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -verify-machineinstrs \
+; RUN:   -mattr=-fpregs,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m33 -mattr=-fpregs -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m35p -mattr=-fpregs -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -verify-machineinstrs \
+; RUN:   -mattr=+fp-armv8d16sp,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mcpu=cortex-m55 -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -verify-machineinstrs \
+; RUN:   -mattr=-fpregs,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mcpu=cortex-m55 -mattr=-fpregs -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+
+define void @non_secure_call(void ()* %fptr) {
+; CHECK-8M-FP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-FP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-FP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:msr apsr_nzcvq{{g?}}, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-FP-CVE-2021-35465-NEXT:tst.w r12, #8
+;

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-15 Thread Sam Elliott via Phabricator via cfe-commits 
lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.

LGTM, but the most recent way of implementing this (using target features) was 
something I suggested to Alexandros based on @ostannard's feedback about LTO. I 
think it is cleaner, and this patch is good, but a re-review from others would 
be helpful.


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-15 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea updated this revision to Diff 372677.
labrinea added a comment.

Changes in this revision:

- added `-verify-machineinstrs` to the tests
- that yield two bugs that I had to address:

  *** Bad machine code: Explicit operand marked as def ***
  - function:func
  - basic block: %bb.0 entry (0x890b6d8)
  - instruction: $d3 = VSTRD $sp, 6, 14, $noreg
  - operand 0:   $d3



  *** Bad machine code: Explicit definition marked as use ***
  - function:non_secure_call
  - basic block: %bb.0  (0x8e0bed8)
  - instruction: t2MRS_M $r12, 20, 14, $noreg
  - operand 0:   $r12


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

https://reviews.llvm.org/D109157

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-cmse-cve-2021-35465.c
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
  llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir

Index: llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
===
--- llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
+++ llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
@@ -1,4 +1,4 @@
-# RUN: llc -mtriple=thumbv8m.main -mcpu=cortex-m33 --float-abi=hard --run-pass=arm-pseudo %s -o - | \
+# RUN: llc -mtriple=thumbv8m.main -mcpu=cortex-m33 -mattr=-fix-cmse-cve-2021-35465 --float-abi=hard --run-pass=arm-pseudo %s -o - | \
 # RUN: FileCheck %s
 --- |
   ; ModuleID = 'cmse-vlldm-no-reorder.ll'
@@ -109,4 +109,4 @@
 # CHECK-NEXT:  $s0 = VMOVSR $r12, 14 /* CC::al */, $noreg
 # CHECK-NEXT:  $sp = tADDspi $sp, 34, 14 /* CC::al */, $noreg
 # CHECK-NEXT:  $sp = t2LDMIA_UPD $sp, 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $r7, def $r8, def $r9, def $r10, def $r11
- 
\ No newline at end of file
+ 
Index: llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
@@ -0,0 +1,119 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -verify-machineinstrs \
+; RUN:   -mattr=+fp-armv8d16sp,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m33 -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m35p -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -verify-machineinstrs \
+; RUN:   -mattr=-fpregs,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m33 -mattr=-fpregs -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m35p -mattr=-fpregs -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -verify-machineinstrs \
+; RUN:   -mattr=+fp-armv8d16sp,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mcpu=cortex-m55 -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -verify-machineinstrs \
+; RUN:   -mattr=-fpregs,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mcpu=cortex-m55 -mattr=-fpregs -verify-machineinstrs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+
+define void @non_secure_call(void ()* %fptr) {
+; CHECK-8M-FP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-FP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-FP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r10, r0
+;

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-15 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea updated this revision to Diff 372674.
labrinea added a comment.

Changes in this revision:

- Replaced the backend option that enables the mitigation with a subtarget 
feature so that it works with LTO (@lenary thanks for the offline hint)
- Enabled the subtarget feature on the affected CPUs


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

https://reviews.llvm.org/D109157

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-cmse-cve-2021-35465.c
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
  llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir

Index: llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
===
--- llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
+++ llvm/test/CodeGen/ARM/cmse-vlldm-no-reorder.mir
@@ -1,4 +1,4 @@
-# RUN: llc -mtriple=thumbv8m.main -mcpu=cortex-m33 --float-abi=hard --run-pass=arm-pseudo %s -o - | \
+# RUN: llc -mtriple=thumbv8m.main -mcpu=cortex-m33 -mattr=-fix-cmse-cve-2021-35465 --float-abi=hard --run-pass=arm-pseudo %s -o - | \
 # RUN: FileCheck %s
 --- |
   ; ModuleID = 'cmse-vlldm-no-reorder.ll'
@@ -109,4 +109,4 @@
 # CHECK-NEXT:  $s0 = VMOVSR $r12, 14 /* CC::al */, $noreg
 # CHECK-NEXT:  $sp = tADDspi $sp, 34, 14 /* CC::al */, $noreg
 # CHECK-NEXT:  $sp = t2LDMIA_UPD $sp, 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $r7, def $r8, def $r9, def $r10, def $r11
- 
\ No newline at end of file
+ 
Index: llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
@@ -0,0 +1,119 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main \
+; RUN:   -mattr=+fp-armv8d16sp,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m33 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m35p | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main \
+; RUN:   -mattr=-fpregs,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m33 -mattr=-fpregs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mcpu=cortex-m35p -mattr=-fpregs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main \
+; RUN:   -mattr=+fp-armv8d16sp,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mcpu=cortex-m55 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main \
+; RUN:   -mattr=-fpregs,+fix-cmse-cve-2021-35465 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mcpu=cortex-m55 -mattr=-fpregs | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+
+define void @non_secure_call(void ()* %fptr) {
+; CHECK-8M-FP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-FP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-FP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:msr apsr_nzcvq{{g?}}, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-FP-CVE-2021-35465-NEXT:tst.w r12, #8
+; CHECK-8M-FP-CVE-2021-35465-NEXT:it ne
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vmovne.f32 s0, s0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlldm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:add sp, #136
+;

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-14 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea added a comment.

@ostannard, can you explain what you meant with supporting LTO? I didn't quite 
undestand. Are you happy with the rest of the changes?


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-08 Thread Sjoerd Meijer via Phabricator via cfe-commits 
SjoerdMeijer added inline comments.



Comment at: llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll:6
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+
+%indirect = type { double, double, double, double, double, double, double, 
double }

As you're creating machine instructions, I think it is better to run this and 
the other test with `-verify-machineinstrs.`


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-07 Thread Sjoerd Meijer via Phabricator via cfe-commits 
SjoerdMeijer added a comment.

The driver part looks good to me. I will let Oli do the approval as he also 
looked at the codegen (and I didn't).




Comment at: clang/include/clang/Driver/Options.td:3274
+  Group,
+  HelpText<"Work around VLLDM erratum CVE-2021-35465 (Arm only)">;
+def mno_fix_cmse_cve_2021_35465 : Flag<["-"], "mno-fix-cmse-cve-2021-35465">,

Nit: I think `Arm` -> `ARM` is better as we we're talking about the ARM backend 
here.


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-07 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea updated this revision to Diff 371083.
labrinea added a comment.

Changes in this revision:

- renamed fix_cve_2021_35465 to Fix_CVE_2021_35465
- added more Driver tests to cover the use of -mfix-cmse-cve-2021-35465 with 
-mno-fix-cmse-cve-2021-35465
- fixed code indentation


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

https://reviews.llvm.org/D109157

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/arm-cmse-cve-2021-35465.c
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll

Index: llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
@@ -0,0 +1,101 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mattr=+fp-armv8d16sp \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mattr=-fpregs \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mattr=+fp-armv8d16sp \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mattr=-fpregs \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+
+
+define void @non_secure_call(void ()* %fptr) {
+; CHECK-8M-FP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-FP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-FP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:msr apsr_nzcvq, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-FP-CVE-2021-35465-NEXT:tst.w r12, #8
+; CHECK-8M-FP-CVE-2021-35465-NEXT:it ne
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vmovne.f32 s0, s0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlldm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:add sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:pop.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:pop {r7, pc}
+;
+; CHECK-8M-NOFP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-NOFP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:msr apsr_nzcvq, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:tst.w r12, #8
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:it ne
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:@APP
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:.inst.w 0xeeb00a40
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:@NO_APP
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:vlldm sp
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:add sp, #136
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:pop.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:pop {r7, pc}
+;
+;

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-07 Thread Sjoerd Meijer via Phabricator via cfe-commits 
SjoerdMeijer added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))
+  CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1");

labrinea wrote:
> SjoerdMeijer wrote:
> > I am wondering if this should use `getLastArg` and what happens with test 
> > cases (which I guess need adding) that have both:
> > 
> >   -mno-fix-cmse-cve-2021-35465  -mfix-cmse-cve-2021-35465 
> > 
> > or
> > 
> >   -mfix-cmse-cve-2021-35465  -mno-fix-cmse-cve-2021-35465
> That's the whole point of `getLastArg` as far as I understand: for options 
> that can either enable or disable a feature, so that the last one wins. I'll 
> add more tests.
Ah, got confused, it's indeed right there!


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-07 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea marked 4 inline comments as done and 6 inline comments as done.
labrinea added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))
+  CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1");

SjoerdMeijer wrote:
> I am wondering if this should use `getLastArg` and what happens with test 
> cases (which I guess need adding) that have both:
> 
>   -mno-fix-cmse-cve-2021-35465  -mfix-cmse-cve-2021-35465 
> 
> or
> 
>   -mfix-cmse-cve-2021-35465  -mno-fix-cmse-cve-2021-35465
That's the whole point of `getLastArg` as far as I understand: for options that 
can either enable or disable a feature, so that the last one wins. I'll add 
more tests.


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-07 Thread Sjoerd Meijer via Phabricator via cfe-commits 
SjoerdMeijer added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1646
 
-  if (Args.getLastArg(options::OPT_mcmse))
+  bool fix_cve_2021_35465 = false;
+  if (Args.getLastArg(options::OPT_mcmse)) {

Nit: capital F in variable name.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))
+  CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1");

I am wondering if this should use `getLastArg` and what happens with test cases 
(which I guess need adding) that have both:

  -mno-fix-cmse-cve-2021-35465  -mfix-cmse-cve-2021-35465 

or

  -mfix-cmse-cve-2021-35465  -mno-fix-cmse-cve-2021-35465


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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-07 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea updated this revision to Diff 371028.
labrinea added a comment.

Changes in this revision:

- pass the -arm-fix-cmse-cve-2021-35465 option once
- document -m(no)fix-cmse-cve-2021-35465 in ClangCommandLineReference.rst
- add clang tests with the mitigation expicitely disabled on affected cpus
- removed `.addReg(ARM::CPSR, RegState::ImplicitDefine)`
- moved the mitigation sequence inside a bundle to prevent reordering


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

https://reviews.llvm.org/D109157

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/arm-cmse-cve-2021-35465.c
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll

Index: llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
@@ -0,0 +1,101 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mattr=+fp-armv8d16sp \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mattr=-fpregs \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mattr=+fp-armv8d16sp \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mattr=-fpregs \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+
+
+define void @non_secure_call(void ()* %fptr) {
+; CHECK-8M-FP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-FP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-FP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:msr apsr_nzcvq, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-FP-CVE-2021-35465-NEXT:tst.w r12, #8
+; CHECK-8M-FP-CVE-2021-35465-NEXT:it ne
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vmovne.f32 s0, s0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlldm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:add sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:pop.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:pop {r7, pc}
+;
+; CHECK-8M-NOFP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-NOFP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:msr apsr_nzcvq, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:tst.w r12, #8
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:it ne
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:@APP
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:.inst.w 0xeeb00a40
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:@NO_APP
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:vlldm sp
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:add sp, #136

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-06 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665
+
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))

ostannard wrote:
> ostannard wrote:
> > labrinea wrote:
> > > ostannard wrote:
> > > > Are these optional also being passed through to the linker when doing 
> > > > LTO?
> > > No, the mitigation is only performed in the compiler. Also, I believe 
> > > that `-flto` and `-mcmse` are incompatible options.
> > The mitigation is performed in the backend, which is run from the linker 
> > when doing LTO.
> > 
> > > Also, I believe that -flto and -mcmse are incompatible options.
> > 
> > Fair enough
> Actually, I just checked and these options are accepted together, and I can't 
> find any docs saying they are incompatible. Do you have a link to something 
> I've missed? Since there isn't already an error, I think we should either fix 
> this to work with LTO (my preference), or add an error when using the options 
> together, and document that.
I have addressed all the other comments, but I am not sure how to go about this 
one. What does it take to make the cve-2021-35465 option work with LTO? Could 
you elaborate on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits 
ostannard added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665
+
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))

ostannard wrote:
> labrinea wrote:
> > ostannard wrote:
> > > Are these optional also being passed through to the linker when doing LTO?
> > No, the mitigation is only performed in the compiler. Also, I believe that 
> > `-flto` and `-mcmse` are incompatible options.
> The mitigation is performed in the backend, which is run from the linker when 
> doing LTO.
> 
> > Also, I believe that -flto and -mcmse are incompatible options.
> 
> Fair enough
Actually, I just checked and these options are accepted together, and I can't 
find any docs saying they are incompatible. Do you have a link to something 
I've missed? Since there isn't already an error, I think we should either fix 
this to work with LTO (my preference), or add an error when using the options 
together, and document that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits 
ostannard added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665
+
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))

labrinea wrote:
> ostannard wrote:
> > Are these optional also being passed through to the linker when doing LTO?
> No, the mitigation is only performed in the compiler. Also, I believe that 
> `-flto` and `-mcmse` are incompatible options.
The mitigation is performed in the backend, which is run from the linker when 
doing LTO.

> Also, I believe that -flto and -mcmse are incompatible options.

Fair enough



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))
+  CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1");

labrinea wrote:
> SjoerdMeijer wrote:
> > If `-mcpu=cortex-[m33|m35|m55]` was provided, then 
> > `-arm-fix-cmse-cve-2021-35465=1` is already set and we are adding another 
> > option here? For example, for
> > 
> >   -mcpu=cortex-m33 -mcmse -mfix-cmse-cve-2021-35465
> > 
> > I am expecting:
> > 
> >   "-mllvm" "-arm-fix-cmse-cve-2021-35465=1"  "-mllvm" 
> > "-arm-fix-cmse-cve-2021-35465=1" 
> > 
> > and with `-mno-fix-cmse-cve-2021-35465`:
> > 
> >"-mllvm" "-arm-fix-cmse-cve-2021-35465=1"  "-mllvm" 
> > "-arm-fix-cmse-cve-2021-35465=0" 
> > 
> > Probably it's nicer to just pass this once.
> > 
> > Also, in the tests, I think cases are missing for `-mcpu=...` and 
> > `-m[no-]fix-cmse-cve-2021-35465`.
> Your interpretetion is correct. The established policy is "last option wins", 
> but I could make the Driver pass only one option if that's preferable.
I agree with Sjoerd, the ""last option wins" policy should be implemented in 
the driver, and only the winning option passed through to CC1. You can use 
`Args.hasFlag` instead of `getLastArg` here, with the default set based on the 
CPU option.



Comment at: clang/test/Driver/arm-cmse-cve-2021-35465.c:3
+//
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8-m.main %s -### \
+// RUN:   -mcmse -mno-fix-cmse-cve-2021-35465 2>&1 |\

labrinea wrote:
> ostannard wrote:
> > The last few paragraphs of 
> > https://developer.arm.com/support/arm-security-updates/vlldm-instruction-security-vulnerability
> >  claim that this is enabled by default for -march=armv8-m.main in AC6 and 
> > GCC, is there a reason we're not matching that?
> Yes, the inconsistency lies on the fact that GCC implements the mitigation in 
> library code, therefore it is always present for `-march=armv8-m.main`, 
> whereas in llvm there's no such limitation. We've contacted the authors of 
> this page to rectify the documentation.
This still sounds like an inconsistency which will catch out users migrating 
between GCC and clang. I'd prefer that we match GCC's behaviour, though I'd 
also be OK with leaving it like this as long as these defaults are clearly 
documented in ClangCommandLineReference.rst.



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1564
   .addReg(Reg)
+  .addReg(ARM::CPSR, RegState::ImplicitDefine)
   .add(predOps(ARMCC::AL));

labrinea wrote:
> ostannard wrote:
> > Why are these needed?
> These prevent the reordering with the mitigation sequence. It answers your 
> next question.
Do you mean that this is modeling the effect of this instruction on the CONTROL 
register, to prevent it from being re-ordered with the MRS instruction? If so, 
then this is unrelated to my next question, and could do with a comment because 
I wouldn't expect any CONTROL bits to be part of ARM::CPSR (they are different 
registers).



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1626
+  // Thumb2ITBlockPass will not recognise the instruction as conditional.
+  BuildMI(MBB, MBBI, DL, TII->get(ARM::t2IT))
+  .addImm(ARMCC::NE)

labrinea wrote:
> ostannard wrote:
> > This pass runs before the final scheduler pass, so is there a risk that the 
> > IT and VMOV instructions could be moved apart? I think it would be safer to 
> > either put the IT instruction inside the inline asm block, or make this a 
> > new pseudo-instruction expanded in the asm printer.
> The use of `.addReg(ARM::CPSR, RegState::ImplicitDefine)` prevents the 
> reordering. There are regression tests in place that check the mitigation 
> sequence ordering.
> 
> Is this what you meant? Where you refering specifically to the case where 
> `!STI->hasFPRegs()`, when we generate inline asm, or to both scenarios?
My concern is that these two instructions (IT and VMOV) have to be adjacent, 
otherwise the IT would apply to some other instruction, but I don't think 
there's anything here which guarantees that. I don't think a regression test is 
enough here, because the re-ordering

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-06 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665
+
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))

ostannard wrote:
> Are these optional also being passed through to the linker when doing LTO?
No, the mitigation is only performed in the compiler. Also, I believe that 
`-flto` and `-mcmse` are incompatible options.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))
+  CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1");

SjoerdMeijer wrote:
> If `-mcpu=cortex-[m33|m35|m55]` was provided, then 
> `-arm-fix-cmse-cve-2021-35465=1` is already set and we are adding another 
> option here? For example, for
> 
>   -mcpu=cortex-m33 -mcmse -mfix-cmse-cve-2021-35465
> 
> I am expecting:
> 
>   "-mllvm" "-arm-fix-cmse-cve-2021-35465=1"  "-mllvm" 
> "-arm-fix-cmse-cve-2021-35465=1" 
> 
> and with `-mno-fix-cmse-cve-2021-35465`:
> 
>"-mllvm" "-arm-fix-cmse-cve-2021-35465=1"  "-mllvm" 
> "-arm-fix-cmse-cve-2021-35465=0" 
> 
> Probably it's nicer to just pass this once.
> 
> Also, in the tests, I think cases are missing for `-mcpu=...` and 
> `-m[no-]fix-cmse-cve-2021-35465`.
Your interpretetion is correct. The established policy is "last option wins", 
but I could make the Driver pass only one option if that's preferable.



Comment at: clang/test/Driver/arm-cmse-cve-2021-35465.c:3
+//
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8-m.main %s -### \
+// RUN:   -mcmse -mno-fix-cmse-cve-2021-35465 2>&1 |\

ostannard wrote:
> The last few paragraphs of 
> https://developer.arm.com/support/arm-security-updates/vlldm-instruction-security-vulnerability
>  claim that this is enabled by default for -march=armv8-m.main in AC6 and 
> GCC, is there a reason we're not matching that?
Yes, the inconsistency lies on the fact that GCC implements the mitigation in 
library code, therefore it is always present for `-march=armv8-m.main`, whereas 
in llvm there's no such limitation. We've contacted the authors of this page to 
rectify the documentation.



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1564
   .addReg(Reg)
+  .addReg(ARM::CPSR, RegState::ImplicitDefine)
   .add(predOps(ARMCC::AL));

ostannard wrote:
> Why are these needed?
These prevent the reordering with the mitigation sequence. It answers your next 
question.



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1626
+  // Thumb2ITBlockPass will not recognise the instruction as conditional.
+  BuildMI(MBB, MBBI, DL, TII->get(ARM::t2IT))
+  .addImm(ARMCC::NE)

ostannard wrote:
> This pass runs before the final scheduler pass, so is there a risk that the 
> IT and VMOV instructions could be moved apart? I think it would be safer to 
> either put the IT instruction inside the inline asm block, or make this a new 
> pseudo-instruction expanded in the asm printer.
The use of `.addReg(ARM::CPSR, RegState::ImplicitDefine)` prevents the 
reordering. There are regression tests in place that check the mitigation 
sequence ordering.

Is this what you meant? Where you refering specifically to the case where 
`!STI->hasFPRegs()`, when we generate inline asm, or to both scenarios?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-06 Thread Sjoerd Meijer via Phabricator via cfe-commits 
SjoerdMeijer added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1666
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))
+  CmdArgs.push_back("-arm-fix-cmse-cve-2021-35465=1");

If `-mcpu=cortex-[m33|m35|m55]` was provided, then 
`-arm-fix-cmse-cve-2021-35465=1` is already set and we are adding another 
option here? For example, for

  -mcpu=cortex-m33 -mcmse -mfix-cmse-cve-2021-35465

I am expecting:

  "-mllvm" "-arm-fix-cmse-cve-2021-35465=1"  "-mllvm" 
"-arm-fix-cmse-cve-2021-35465=1" 

and with `-mno-fix-cmse-cve-2021-35465`:

   "-mllvm" "-arm-fix-cmse-cve-2021-35465=1"  "-mllvm" 
"-arm-fix-cmse-cve-2021-35465=0" 

Probably it's nicer to just pass this once.

Also, in the tests, I think cases are missing for `-mcpu=...` and 
`-m[no-]fix-cmse-cve-2021-35465`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-06 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits 
ostannard added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1665
+
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_mfix_cmse_cve_2021_35465))

Are these optional also being passed through to the linker when doing LTO?



Comment at: clang/test/Driver/arm-cmse-cve-2021-35465.c:3
+//
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8-m.main %s -### \
+// RUN:   -mcmse -mno-fix-cmse-cve-2021-35465 2>&1 |\

The last few paragraphs of 
https://developer.arm.com/support/arm-security-updates/vlldm-instruction-security-vulnerability
 claim that this is enabled by default for -march=armv8-m.main in AC6 and GCC, 
is there a reason we're not matching that?



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1564
   .addReg(Reg)
+  .addReg(ARM::CPSR, RegState::ImplicitDefine)
   .add(predOps(ARMCC::AL));

Why are these needed?



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1626
+  // Thumb2ITBlockPass will not recognise the instruction as conditional.
+  BuildMI(MBB, MBBI, DL, TII->get(ARM::t2IT))
+  .addImm(ARMCC::NE)

This pass runs before the final scheduler pass, so is there a risk that the IT 
and VMOV instructions could be moved apart? I think it would be safer to either 
put the IT instruction inside the inline asm block, or make this a new 
pseudo-instruction expanded in the asm printer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109157

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

[PATCH] D109157: [ARM] Mitigate the cve-2021-35465 security vulnurability.

2021-09-02 Thread Alexandros Lamprineas via Phabricator via cfe-commits 
labrinea created this revision.
labrinea added reviewers: llvm-commits, momchil.velikov.
Herald added subscribers: dang, hiraditya, kristof.beyls.
labrinea requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

Recently a vulnerability issue is found in the implementation of VLLDM 
instruction in the Arm Cortex-M33, Cortex-M35P and Cortex-M55. If the VLLDM 
instruction is abandoned due to an exception when it is partially completed, it 
is possible for subsequent non-secure handler to access and modify the partial 
restored register values. This vulnerability is identified as CVE-2021-35465. 
The mitigation sequence varies between v8-m and v8.1-m as follows:

v8-m.main

  mrsr5, control
  tstr5, #8   /* CONTROL_S.SFPA */
  it ne
  .inst.w0xeeb00a40   /* vmovne s0, s0 */
  1:
  vlldm  sp   /* Lazy restore of d0-d16 and FPSCR. */

v8.1-m.main

  vscclrm{vpr}/* Clear VPR. */
  vlldm  sp   /* Lazy restore of d0-d16 and FPSCR. */

More details on 
https://developer.arm.com/support/arm-security-updates/vlldm-instruction-security-vulnerability


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109157

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/arm-cmse-cve-2021-35465.c
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465-return.ll
  llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll

Index: llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/cmse-cve-2021-35465.ll
@@ -0,0 +1,101 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mattr=+fp-armv8d16sp \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-FP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8m.main -mattr=-fpregs \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-8M-NOFP-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mattr=+fp-armv8d16sp \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+;
+; RUN: llc %s -o - -mtriple=thumbv8.1m.main -mattr=-fpregs \
+; RUN:   -arm-fix-cmse-cve-2021-35465=1 | \
+; RUN:   FileCheck %s --check-prefix=CHECK-81M-CVE-2021-35465
+
+
+define void @non_secure_call(void ()* %fptr) {
+; CHECK-8M-FP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-FP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-FP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r9, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r10, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r11, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mov r12, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:msr apsr_nzcvq, r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:blxns r0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:mrs r12, control
+; CHECK-8M-FP-CVE-2021-35465-NEXT:tst.w r12, #8
+; CHECK-8M-FP-CVE-2021-35465-NEXT:it ne
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vmovne.f32 s0, s0
+; CHECK-8M-FP-CVE-2021-35465-NEXT:vlldm sp
+; CHECK-8M-FP-CVE-2021-35465-NEXT:add sp, #136
+; CHECK-8M-FP-CVE-2021-35465-NEXT:pop.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-FP-CVE-2021-35465-NEXT:pop {r7, pc}
+;
+; CHECK-8M-NOFP-CVE-2021-35465-LABEL: non_secure_call:
+; CHECK-8M-NOFP-CVE-2021-35465:   @ %bb.0:
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:push {r7, lr}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:push.w {r4, r5, r6, r7, r8, r9, r10, r11}
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:bic r0, r0, #1
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:sub sp, #136
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:vlstm sp
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r1, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r2, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r3, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r4, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r5, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r6, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r7, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r8, r0
+; CHECK-8M-NOFP-CVE-2021-35465-NEXT:mov r9, r0
+;