Hi,
__builtin_cmse_nonsecure_caller implementation returns true in almost
all cases due to 2 separate bugs:
* gen_addsi is used instead of gen_andsi to retrieve the lsb
* the lsb boolean value is not negated but the specification [1] says
the intrinsic should return true for a nonsecure caller and a
nonsecure caller is characterized with LR's lsb being 0
This was not caught due to (1) lack of runtime test and (2) the existing
RTL scan not taking into account that '.' matches newline in Tcl regular
expressions.
This patch fixes the implementation issues and improves testing of
cmse_nonsecure_caller by (1) adding a runtime test for the secure caller
case and (2) looking for an SET insn of an AND expression in the right
function. This leaves the nonsecure caller case only partly tested
since the exact value being AND and the negation are not covered by the
scan and the existing test infrastructure does not allow 2 separate
compilation and link to be performed. It is enough though to catch the
current incorrect behavior.
The patch also reorganize the scan directives in cmse-1.c to more easily
identify what function they are intended to test in the file.
ChangeLog entry is as follows:
*** gcc/ChangeLog ***
2018-04-04 Thomas Preud'homme <thomas.preudho...@arm.com>
PR target/85203
* config/arm/arm-builtins.c (arm_expand_builtin): Change
expansion to perform a bitwise AND of the argument followed by a
boolean negation of the result.
*** gcc/testsuite/ChangeLog ***
2018-04-04 Thomas Preud'homme <thomas.preudho...@arm.com>
PR target/85203
* gcc.target/arm/cmse/cmse-1.c: Tighten cmse_nonsecure_caller RTL scan
to match a single insn of the baz function. Move scan directives at
the end of the file below the functions they are trying to test for
better readability.
* gcc.target/arm/cmse/cmse-16.c: New testcase.
Testing: No bootstrap since only M profile builtin code has been changed
but regression testing for arm-none-eabi targeting Arm Cortex-M23 and
Cortex-M33 shows no regression.
Is this ok for stage4?
Best regards,
Thomas
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c
index 8940d1f6311bccf86664ab2eaa938735eec595f6..184eb2a934308717b6e1054e376487a297f8d5de 100644
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -2600,7 +2600,9 @@ arm_expand_builtin (tree exp,
case ARM_BUILTIN_CMSE_NONSECURE_CALLER:
target = gen_reg_rtx (SImode);
op0 = arm_return_addr (0, NULL_RTX);
- emit_insn (gen_addsi3 (target, op0, const1_rtx));
+ emit_insn (gen_andsi3 (target, op0, const1_rtx));
+ op1 = gen_rtx_EQ (SImode, target, const0_rtx);
+ emit_insn (gen_cstoresi4 (target, op1, target, const0_rtx));
return target;
case ARM_BUILTIN_TEXTRMSB:
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-1.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-1.c
index c13272eed683aa06db027cd4646e5fe67817212b..f764153cb17b796ccd0d20abb78d5cf56be52911 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-1.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-1.c
@@ -71,6 +71,20 @@ baz (void)
{
return cmse_nonsecure_caller ();
}
+/* { dg-final { scan-assembler "baz:" } } */
+/* { dg-final { scan-assembler "__acle_se_baz:" } } */
+/* { dg-final { scan-assembler-not "\tcmse_nonsecure_caller" } } */
+/* Look for an andsi of 1 with a register in function baz, ie.
+
+;; Function baz<anything>
+<any line without '('>
+(insn <anything but '('> (set (reg<any register modifier>:SI <anything but ')'>)
+ (and:SI (reg<any register modifier>:SI <anything but ')'>)
+ (const_int 1 <anything but ')'>)<anything but '('>
+ <optional: (nil)<anything but '('>>
+(insn
+*/
+/* { dg-final { scan-rtl-dump "\n;; Function baz\[^\n\]*\[^(\]+\[^;\]*\n\\(insn \[^(\]+ \\(set \\(reg\[^:\]*:SI \[^)\]+\\)\[^(\]*\\(and:SI \\(reg\[^:\]*:SI \[^)\]+\\)\[^(\]*\\((const_int 1|reg\[^:\]*:SI) \[^)\]+\\)\[^(\]+(\\(nil\\)\[^(\]+)?\\(insn" expand } } */
typedef int __attribute__ ((cmse_nonsecure_call)) (int_nsfunc_t) (void);
@@ -86,6 +100,11 @@ qux (int_nsfunc_t * callback)
{
fp = cmse_nsfptr_create (callback);
}
+/* { dg-final { scan-assembler "qux:" } } */
+/* { dg-final { scan-assembler "__acle_se_qux:" } } */
+/* { dg-final { scan-assembler "bic" } } */
+/* { dg-final { scan-assembler "push\t\{r4, r5, r6" } } */
+/* { dg-final { scan-assembler "msr\tAPSR_nzcvq" } } */
int call_callback (void)
{
@@ -94,13 +113,4 @@ int call_callback (void)
else
return default_callback ();
}
-/* { dg-final { scan-assembler "baz:" } } */
-/* { dg-final { scan-assembler "__acle_se_baz:" } } */
-/* { dg-final { scan-assembler "qux:" } } */
-/* { dg-final { scan-assembler "__acle_se_qux:" } } */
-/* { dg-final { scan-assembler-not "\tcmse_nonsecure_caller" } } */
-/* { dg-final { scan-rtl-dump "and.*reg.*const_int 1" expand } } */
-/* { dg-final { scan-assembler "bic" } } */
-/* { dg-final { scan-assembler "push\t\{r4, r5, r6" } } */
-/* { dg-final { scan-assembler "msr\tAPSR_nzcvq" } } */
/* { dg-final { scan-assembler-times "bl\\s+__gnu_cmse_nonsecure_call" 1 } } */
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-16.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-16.c
new file mode 100644
index 0000000000000000000000000000000000000000..3fb0380afaafa506cdb68f4d3c1dd7b535d43f26
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-16.c
@@ -0,0 +1,18 @@
+/* { dg-do run } */
+/* { dg-options "-Os -mcmse" } */
+
+#include <arm_cmse.h>
+
+int
+foo (void)
+{
+ return cmse_nonsecure_caller ();
+}
+
+int
+main (void)
+{
+ /* Return success (0) if main is secure, ie if cmse_nonsecure_caller/foo
+ returns false (0). */
+ return foo ();
+}
