Bug 85203 - cmse_nonsecure_caller intrinsic returns incorrect results
Summary: cmse_nonsecure_caller intrinsic returns incorrect results
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.3.1
: P3 normal
Target Milestone: 7.4
Assignee: Thomas Preud'homme
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2018-04-04 14:49 UTC by Thomas Preud'homme
Modified: 2018-04-17 16:21 UTC (History)
1 user (show)

See Also:
Host:
Target: arm-none-eabi
Build:
Known to work: 7.3.1, 8.0.1
Known to fail: 7.3.0
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Preud'homme 2018-04-04 14:49:18 UTC
Hi,

The cmse_nonsecure_caller intrinsic for Armv8-M Baseline and Mainline architecture returns true in almost all cases, ie. compiling and running the following test with arm-none-eabi-gcc -Os -mcmse -march=armv8-m.main will return an error:

#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 ();
}

Looking at the implementation of the associated __builtin_cmse_nonsecure_caller in gcc/config/arm/arm-builtins.c we can see why:

* it performs an add instead of an and to get the lsb of LR
* it does not negate the value to return true when the lsb is 0

This means that except for 0xffffffff it will always return true. I'm currently testing a patch as I write these lines.
Comment 1 Thomas Preud'homme 2018-04-04 17:32:18 UTC
Author: thopre01
Date: Wed Apr  4 17:31:46 2018
New Revision: 259097

URL: https://gcc.gnu.org/viewcvs?rev=259097&root=gcc&view=rev
Log:
[ARM] Fix PR85203: cmse_nonsecure_caller returns wrong result

__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 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 commit 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 commit also reorganize the scan directives in cmse-1.c to more
easily identify what function they are intended to test in the file.

2018-04-04  Thomas Preud'homme  <thomas.preudhomme@arm.com>

gcc/
    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/
    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.

Added:
    trunk/gcc/testsuite/gcc.target/arm/cmse/cmse-16.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm-builtins.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/arm/cmse/cmse-1.c
Comment 2 Thomas Preud'homme 2018-04-05 15:46:39 UTC
Fixed in trunk
Comment 3 Thomas Preud'homme 2018-04-11 09:47:53 UTC
Author: thopre01
Date: Wed Apr 11 09:47:21 2018
New Revision: 259309

URL: https://gcc.gnu.org/viewcvs?rev=259309&root=gcc&view=rev
Log:
[ARM] Fix PR85203: cmse_nonsecure_caller returns wrong result

__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 commit 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 commit also reorganize the scan directives in cmse-1.c to more
easily identify what function they are intended to test in the file.

2018-04-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    Backport from mainline
    2018-04-04  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    gcc/
    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/
    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.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/cmse/cmse-16.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/arm/arm-builtins.c
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
    branches/gcc-7-branch/gcc/testsuite/gcc.target/arm/cmse/cmse-1.c
Comment 4 Ramana Radhakrishnan 2018-04-17 15:33:15 UTC
Fixed I'm assuming ?