bpo-32962: Fix test_gdb failure in debug build with -mcet -fcf-protection -O0 by vstinner · Pull Request #9656 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation12 Commits5 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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

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 gmarcel.plch@gmail.com

(cherry picked from commit 9b7c74c)

@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.

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 ;-)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

CET_CF_PROTECTION = cet_cf_protection()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Pick just one.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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 <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>C</mi><mi>F</mi><mi>L</mi><mi>A</mi><mi>G</mi><mi>S</mi><mi mathvariant="normal">&quot;</mi><mo>&gt;</mo><mo>&gt;</mo></mrow><annotation encoding="application/x-tex">CFLAGS&quot; &gt;&gt; </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:0.7335em;vertical-align:-0.0391em;"></span><span class="mord mathnormal" style="margin-right:0.13889em;">CF</span><span class="mord mathnormal">L</span><span class="mord mathnormal">A</span><span class="mord mathnormal" style="margin-right:0.05764em;">GS</span><span class="mord">&quot;</span><span class="mspace" style="margin-right:0.2778em;"></span><span class="mrel">&gt;&gt;</span></span></span></span>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.

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 9, 2018

…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 gmarcel.plch@gmail.com

(cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133)

Co-authored-by: Victor Stinner vstinner@redhat.com

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 9, 2018

…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 gmarcel.plch@gmail.com

(cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133)

Co-authored-by: Victor Stinner vstinner@redhat.com

miss-islington added a commit that referenced this pull request

Oct 9, 2018

…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 gmarcel.plch@gmail.com

(cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133)

Co-authored-by: Victor Stinner vstinner@redhat.com

miss-islington added a commit that referenced this pull request

Oct 9, 2018

…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 gmarcel.plch@gmail.com

(cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133)

Co-authored-by: Victor Stinner vstinner@redhat.com

vstinner added a commit that referenced this pull request

Oct 10, 2018

…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 gmarcel.plch@gmail.com

(cherry picked from commit 9b7c74c) (cherry picked from commit 79d2133)