bpo-32962: Fix test_gdb failure in debug build with -mcet -fcf-protection -O0 #9656

vstinner · 2018-10-01T15:59:28Z

When Python is built with the intel control-flow protection flags,
-mcet -fcf-protection, gdb is not able to read the stack without
actually jumping inside the function. This means an extra
'next' command is required to make the $pc (program counter)
enter the function and make the stack of the function exposed to gdb.

https://bugs.python.org/issue32962

the-knights-who-say-ni · 2018-10-01T15:59:31Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

…tion -O0 When Python is built with the intel control-flow protection flags, -mcet -fcf-protection, gdb is not able to read the stack without actually jumping inside the function. This means an extra 'next' command is required to make the $pc (program counter) enter the function and make the stack of the function exposed to gdb. Co-Authored-By: Marcel Plch <[email protected]> (cherry picked from commit 9b7c74c)

vstinner · 2018-10-01T16:06:37Z

@Dormouse759: I modified your PR #6754 to only use the "next" command if Python has been compiled with -mcet -fcf-protection, so it should avoid to break the buildbot. Previously, your change caused an issue on x86 Gentoo Non-Debug with X 3.x and AMD64 Debian PGO 3.x buildbot workers:
https://bugs.python.org/issue32962#msg319669

I kept you as a co-author.

vstinner · 2018-10-01T16:16:54Z

I manually tested on Fedora 28:

# Release, CET
$ ./configure CFLAGS='-mcet -fcf-protection -O0' && make && ./python -m test test_gdb; make clean
(...)
Tests result: SUCCESS

# Debug, CET
$ ./configure --with-pydebug CFLAGS='-mcet -fcf-protection -O0' && make && ./python -m test test_gdb; make clean
Tests result: SUCCESS

# Release (no CET)
$ ./configure && make && ./python -m test test_gdb; make clean
(...)
Tests result: SUCCESS

# Debug (no CET)
$ ./configure --with-pydebug && make && ./python -m test test_gdb; make clean
(...)
Tests result: SUCCESS

I also validated that test_gdb fails without the fix ;-)

ghost · 2018-10-04T13:58:19Z

Lib/test/test_gdb.py

+    cflags = sysconfig.get_config_var('CFLAGS')
+    flags = cflags.split()
+    return (('-mcet' in flags)
+            and any(flag.startswith('-fcf-protection') for flag in flags))


What about:
return '-mcet' in flags and '-fcf-protection' in flags

The option takes an optional argument: --fcf-protection=[full|branch|return|none].

Was this fix tested with these options? I'm afraid that only partial protection might not need the extra 'next'.

ghost · 2018-10-04T14:02:17Z

Lib/test/test_gdb.py

+    return (('-mcet' in flags)
+            and any(flag.startswith('-fcf-protection') for flag in flags))
+
+CET_CF_PROTECTION = cet_cf_protection()


Nitpick:
CET - Control-flow enforcement technology
CF - Control-flow
CET_CF_PROTECTION - Control-flow enforcement technology control-flow protection

Pick just one.

CET_CF_PROTECTION - Control-flow enforcement technology control-flow protection

Oops :-) I renamed the constant.

test_gdb doesn't need "next" with -fcf-protection=none nor -fcf-protection=return.

vstinner · 2018-10-09T14:21:40Z

Was this fix tested with these options? I'm afraid that only partial protection might not need the extra 'next'.

I modified my PR to not use "next" for -fcf-protection=none and -fcf-protection=return options.

I tested the following configurations:

./configure --with-pydebug CFLAGS=-O0 -mcet -fcf-protection
./configure CFLAGS=-mcet -fcf-protection
./configure --with-pydebug CFLAGS=-O0 -mcet -fcf-protection=none
./configure CFLAGS=-mcet -fcf-protection=none
./configure --with-pydebug CFLAGS=-O0 -mcet -fcf-protection=branch
./configure CFLAGS=-mcet -fcf-protection=branch
./configure --with-pydebug CFLAGS=-O0 -mcet -fcf-protection=return
./configure CFLAGS=-mcet -fcf-protection=return
./configure --with-pydebug CFLAGS=-O0 -mcet -fcf-protection=full
./configure CFLAGS=-mcet -fcf-protection=full

using this shell script:

set -e
set -x

LOG_FILE=test.log

function test_python {
    CFLAGS=$2
    make clean
    if [ $1 -eq 0 ]; then
        echo ./configure --with-pydebug CFLAGS="-O0 $CFLAGS" >> $LOG_FILE
        ./configure --with-pydebug CFLAGS="-O0 $CFLAGS"
    else
        echo ./configure CFLAGS="$CFLAGS" >> $LOG_FILE
        ./configure CFLAGS="$CFLAGS"
    fi
    make
    ./python -m test test_gdb
}

for value in "" none branch return full; do
    if [ "$value" != "" ]; then
        CFLAGS="-mcet -fcf-protection=$value"
    else
        CFLAGS="-mcet -fcf-protection"
    fi

    test_python 0 "$CFLAGS"
    test_python 1 "$CFLAGS"
done

test_gdb now pass with all configurations.

miss-islington · 2018-10-09T14:54:10Z

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

…tion -O0 (pythonGH-9656) When Python is built with the intel control-flow protection flags, -mcet -fcf-protection, gdb is not able to read the stack without actually jumping inside the function. This means an extra 'next' command is required to make the $pc (program counter) enter the function and make the stack of the function exposed to gdb. Co-Authored-By: Marcel Plch <[email protected]> (cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133) Co-authored-by: Victor Stinner <[email protected]>

bedevere-bot · 2018-10-09T14:54:32Z

GH-9770 is a backport of this pull request to the 3.7 branch.

…tion -O0 (pythonGH-9656) When Python is built with the intel control-flow protection flags, -mcet -fcf-protection, gdb is not able to read the stack without actually jumping inside the function. This means an extra 'next' command is required to make the $pc (program counter) enter the function and make the stack of the function exposed to gdb. Co-Authored-By: Marcel Plch <[email protected]> (cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133) Co-authored-by: Victor Stinner <[email protected]>

bedevere-bot · 2018-10-09T14:54:39Z

GH-9771 is a backport of this pull request to the 3.6 branch.

…tion -O0 (GH-9656) When Python is built with the intel control-flow protection flags, -mcet -fcf-protection, gdb is not able to read the stack without actually jumping inside the function. This means an extra 'next' command is required to make the $pc (program counter) enter the function and make the stack of the function exposed to gdb. Co-Authored-By: Marcel Plch <[email protected]> (cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133) Co-authored-by: Victor Stinner <[email protected]>

…tion -O0 (GH-9656) (GH-9788) When Python is built with the intel control-flow protection flags, -mcet -fcf-protection, gdb is not able to read the stack without actually jumping inside the function. This means an extra 'next' command is required to make the $pc (program counter) enter the function and make the stack of the function exposed to gdb. test_gdb: get_gdb_repr() now uses the "backtrace 1" command after breakpoint, as in the master branch. Co-Authored-By: Marcel Plch <[email protected]> (cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133)

the-knights-who-say-ni added the CLA not signed label Oct 1, 2018

bedevere-bot added tests Tests in the Lib/test dir awaiting merge labels Oct 1, 2018

vstinner added needs backport to 3.6 and removed awaiting merge tests Tests in the Lib/test dir labels Oct 1, 2018

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Oct 1, 2018

Fix test_gdb if sysconfig.get_config_var() returns None

186d879

ghost reviewed Oct 4, 2018

View reviewed changes

vstinner added 3 commits October 4, 2018 16:48

Rename CET_CF_PROTECTION to CET_PROTECTION

7128b93

Merge branch 'master' into gdb_cet

bc52d26

Handle -fcf-protection=none and -fcf-protection=return

24ecea6

test_gdb doesn't need "next" with -fcf-protection=none nor -fcf-protection=return.

vstinner merged commit 79d2133 into python:master Oct 9, 2018

vstinner deleted the gdb_cet branch October 9, 2018 14:54

bedevere-bot removed the needs backport to 3.7 label Oct 9, 2018

bedevere-bot removed the needs backport to 3.6 label Oct 9, 2018

Uh oh!

bpo-32962: Fix test_gdb failure in debug build with -mcet -fcf-protection -O0 #9656

bpo-32962: Fix test_gdb failure in debug build with -mcet -fcf-protection -O0 #9656

Uh oh!

Conversation

vstinner commented Oct 1, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Oct 1, 2018

Uh oh!

vstinner commented Oct 1, 2018

Uh oh!

vstinner commented Oct 1, 2018

Uh oh!

ghost Oct 4, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 4, 2018

Choose a reason for hiding this comment

Uh oh!

ghost Oct 4, 2018

Choose a reason for hiding this comment

Uh oh!

ghost Oct 4, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 4, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 9, 2018

Uh oh!

miss-islington commented Oct 9, 2018

Uh oh!

bedevere-bot commented Oct 9, 2018

Uh oh!

bedevere-bot commented Oct 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vstinner commented Oct 1, 2018 •

edited by bedevere-bot

Loading

ghost Oct 4, 2018 •

edited by ghost

Loading