New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 654866 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

gcc optimization bug in -ftree-vrp

Project Member Reported by pihsun@chromium.org, Oct 11 2016

Issue description

There's a bug in gcc inside cros_sdk chroot when using optimization -ftree-vrp (which is on when using -O2, which is the default), that cause compiled gpg to behave incorrectly.

How to reproduce:
In cros_sdk, run the following commands:

cat >/tmp/foo <<EOF
  # Key type doesn't matter, just choosing an arbitrary algo.
  Key-Type: DSA
  Key-Length: 1024
  Subkey-Type: ELG-E
  Subkey-Length: 1024
  Name-Real: foo
  Name-Comment: bar
  Name-Email: qux@foo.bar
  Expire-Date: 0
  Passphrase: abc
  %pubring /tmp/foo.pub
  %secring /tmp/foo.sec
  %commit
EOF
gpg --batch --gen-key /tmp/foo
dd if=/dev/urandom of=/tmp/file bs=16K count=1
gpg --output /tmp/file.gpg --no-default-keyring --keyring /tmp/foo.pub --trust-model always --encrypt --recipient foo /tmp/file
gpg --output /tmp/file --no-default-keyring --keyring /tmp/foo.pub --secret-keyring /tmp/foo.sec --trust-model always --passphrase abc --decrypt /tmp/file.gpg

result:
gpg: Segmentation fault caught ... exiting
Segmentation fault (core dumped)

We can verify that the problem is because of -ftree-vrp by running `sudo env CFLAGS='-O2 -pipe -fno-tree-vrp' emerge app-crypt/gnupg`, and the above decrypt would succeed.

The segmentation fault comes from file g10/encr-data.c, function mdc_decode_filter.
Relevant code in line 255-259:
            for(; n < size; n++ ) {
                if( (c = iobuf_get(a)) == -1 )
                    break;
                buf[n] = c;
            }
The compiler incorrectly optimized away the check 'n < size', thus causing array access out of bound.

Attached file is relevant file from compiling gnupg-1.4.15 with -fdump-tree-all, with the bugged gcc inside cros_sdk (gcc.real (4.9.2_cos_gg_4.9.2-r136-fe767fbb6d580458aeea609ac5c82b600acce63d_4.9.2-r136) 4.9.x 20150123 (prerelease)) (files named *.wrong) and with a good gcc (gcc 4.9.2 on goobuntu) (files named *.right).
In encr-data.c.068t.vrp1.wrong, we can see that the value range of n_2 is wrong (line 215), so "n_2 < size_116" is folded into 1 (line 324).
And from the disassembly of encr-data.o.wrong, address 0x1f5-0x219 which corresponds to the loop, there's no exit condition n < size (those 2 jne/jb inside is from macro expansion of iobuf_get).

Tried gcc 4.8.4, 4.9.2, 4.9.4 in goobuntu (4.9.2, 4.9.4 are directly compiled from gcc source), and can't reproduce the issue, so it seems to be specific to cros gcc.
 
encr-data.c.065t.mergephi2.right
18.1 KB Download
encr-data.c.066t.vrp1.right
35.4 KB Download
encr-data.c.067t.mergephi2.wrong
18.2 KB Download
encr-data.c.068t.vrp1.wrong
35.1 KB Download
encr-data.o.right
9.0 KB Download
encr-data.o.wrong
9.1 KB Download

Comment 1 by vapier@chromium.org, Oct 24 2016

Cc: yunlian@chromium.org llozano@chromium.org
Labels: -Restrict-View-Google -OS-Linux Build-Toolchain OS-Chrome
Owner: cmt...@chromium.org
Status: Assigned (was: Untriaged)
caroline, please see if you can find the upstream fix for this. 
is this fixed in the GCC version used by google3?

Comment 3 by cmt...@chromium.org, Sep 25 2017

pihsun@,  I was following your instructions for reproducing the error, but there seems to be some important setup step that's missing.  I did the 'cat' command, the first 'gpg' command, and the 'dd' command.  But the second 'gpg' command is complaining about missing files:


$ gpg --output /tmp/file.gpg --no-default-keyring --keyring /tmp/foo.pub --trust-model always --encrypt --recipient foo /tmp/file
gpg: fatal: can't open `/home/cmtice/.gnupg/trustdb.gpg': No such file or directory
secmem usage: 1408/1408 bytes in 2/2 blocks of pool 1408/32768
$

How do I set this up so I can proceed to reproduce your issue?
do we need to pursue this? 
This package should be built with LLVM now. 
If so, lets not worry about this bug. 

Comment 5 by pihsun@chromium.org, Sep 26 2017

Yes the package is built with LLVM now, so the bug should no longer be relevant.

Comment 6 by cmt...@chromium.org, Sep 26 2017

Status: WontFix (was: Assigned)

Sign in to add a comment