Bug 821918

Summary: pari 2.5.x fails to build with optimization > O0 (-ftree-dse) with gcc 4.7
Product: [Fedora] Fedora Reporter: Paul Howarth <paul>
Component: pariAssignee: Paul Howarth <paul>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 17CC: jakub, law, paul, tremble
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
URL: http://pari.math.u-bordeaux.fr/cgi-bin/bugreport.cgi?bug=1314
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-05-16 17:39:48 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
SRPM showing problem and workaround
none
Fix for scoping issue none

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.