Bug 821918 - pari 2.5.x fails to build with optimization > O0 (-ftree-dse) with gcc 4.7
Summary: pari 2.5.x fails to build with optimization > O0 (-ftree-dse) with gcc 4.7
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: pari
Version: 17
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: Fedora Extras Quality Assurance
URL: http://pari.math.u-bordeaux.fr/cgi-bi...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-05-15 18:39 UTC by Paul Howarth
Modified: 2012-05-16 17:39 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-05-16 17:39:48 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
SRPM showing problem and workaround (2.60 MB, application/x-rpm)
2012-05-15 18:39 UTC, Paul Howarth
no flags Details
Fix for scoping issue (972 bytes, patch)
2012-05-16 17:38 UTC, Paul Howarth
no flags Details | Diff

Description Paul Howarth 2012-05-15 18:39:40 UTC
Created attachment 584761 [details]
SRPM showing problem and workaround

I would like to update pari to 2.5.x but it fails one of the tests in its test suite when built with the default Fedora %{optflags} value, which includes -O2. I have narrowed this failure down to the function vecsort0() in the file pari-2.5.1/src/basemath/bibli2.c and can work around the issue by wrapping the function with an optimization pragma that disables -ftree-dse (Patch13 in the attached SRPM). If the application of this patch is commented out in the spec file, the package fails to build in F-17 and Rawhide (though it builds OK for F-16).

Looking at the intermediate code generated after optimization, it appears that the dead store elimination is removing the assignments:

    v.cmp = cmp;
    v.k = k;

at lines 1442 and 1443 of the original code, and this seems wrong because the next line is:

    E = (void*)&v;

and then E is passed as a parameter to other functions later on in the code, and there are no further assignments to v.

Am I right that this is an optimization bug or is there something wrong with the code that I'm missing?

Comment 1 Jakub Jelinek 2012-05-15 18:55:41 UTC
That is not a gcc bug.

  if (k) {
...
    struct veccmp_s v;
...
    v.cmp = cmp;
    v.k = k;
    E = (void*)&v;
    CMP = &veccmp;    
  } else {
    E = (void*)((typ(x) == t_VECSMALL)? cmp_small: cmp);
    CMP = &cmp_nodata;
  }
...
  use *E

This is undefined behavior, after the v variable goes out of scope (at } before else), you can't reference it any longer, even when the E pointer is still in scope.  It is the same bug as returning an address of an automatic variable from a function:
int *foo (void) { int x = 1; return &x; }

Comment 2 Paul Howarth 2012-05-15 20:52:07 UTC
Excellent - moving the "struct veccmp_s v" declaration up to the top of the function's scope seems to fix this. Thank you very much.

Comment 3 Paul Howarth 2012-05-16 17:38:06 UTC
Created attachment 585026 [details]
Fix for scoping issue

Attached patch sent upstream (http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1314) and should be included in pari 2.5.2.


Note You need to log in before you can comment on or make changes to this bug.