[Bug middle-end/21059] Bogus warning about clobbered variable

[Bug middle-end/21059] Bogus warning about clobbered variable

[gdr at gcc dot gnu dot org]

-- 

gdr at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-11-26 07:39:13
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[wilson at gcc dot gnu dot org]

------- Comment #4 from wilson at gcc dot gnu dot org  2005-12-02 02:24 -------
The spurious warning is a problem because binutils is compiled by -Werror by
default now, when compiled by gcc.  This spurious warning thus causes a build
failure for binutils.  This build failure does not show up in a native
ia64-linux build fortunately, it only shows up for cross builds, or if
--enable-targets=all is used.  Still, this means it is a problem for people
trying to do binutils development on an ia64-linux host.  This may also cause
problems with other programs as use of -Werror spreads.

Also, the spurious warning is a problem because the warning is inherently
unportable.  Different targets and/or different optimization options can result
in different sets of warnings.  So what happens is that one person compiles the
code and gets no warning.  Another compiles the code for a different target,
gets a warning, and submits a patch.  Then the first person points out that the
code isn't broken, that the patch shouldn't necessary, and recommends that the
compiler should be fixed.  Hence this PR.

The failure happens for ia64 because of the lack of reg+offset addressing
modes.  We have a parameter info, copied into pseudo-reg 345, that is
dereferenced twice, once for the fprintf_func field and once for the stream
field.  These fields are at offset 0 and 4.  Normally, we would get two mems
one with address (reg 345) and one with address (plus (reg 345) (const_int 4)).
 But IA-64 does not have reg+offset, so we end up with a post-increment of reg
345.  This means reg 345 is now set twice.  Once before the setjmp, and once
afterwards.  This triggers the code in regno_clobbered_at_setjmp which has a
test "REG_N_SETS (regno) > 1" which is now true for IA-64, but not for most
other targets.

The best fix I see is to improve the CFG to include info about which block
after setjmp is the fallthrough and which is the return from longjmp path. 
Once we have the improved CFG, then we can implement a more accurate test, such
as the one tft mentioned in comment #3.  It may be easier and/or more
appropriate to do this work in the tree-ssa part of the compiler.  This is
likely more work than I can do at the moment for this PR.

A more limited solution may be possible by adjusting the setjmp warning code to
take these post-inc sets into account.  We could keep track of sets created by
auto-inc optimizations, and then subtract them in the setjmp warning code. 
This would ensure the same results for all targets, regardless of addressing
modes.  The flaw here is that if auto-inc opts do modify a register, then we
really do have a problem, and really should have emitted a warning.  So this
doesn't seem worth pursuing.

Alternatively, we could disable auto-inc optimization for pseudo regs that live
across setjmp.  Such pseudo regs won't be allocated to hard regs, so use of
auto-inc addressing modes isn't profitable here anyways.  I think this is a
much better idea.  Unfortunately, it can't be easily implemented, as we can
perform auto-inc optimizations before regs_live_at_setjmp is set.  We would
need another  update_life_info call to do this.  Not clear if this is
worthwhile.  I think it is worth investigating though.

Another alternative is to just remove the warning code.  Most warnings emitted
by this code are probably spurious.  This may not be a viable solution though,
since the warnings are actually useful on some occasions, and the lack of
warnings for problematic code could be considered a regression.  This is
probably not feasible.


-- 

wilson at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilson at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[wilson at gcc dot gnu dot org]

------- Comment #5 from wilson at gcc dot gnu dot org  2005-12-02 04:12 -------
Created an attachment (id=10387)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10387&action=view)
Delay auto-inc generation until after determining which regs live across
setjmp.

This is my proposed untested patch.

While writing this, I realized that we have an actual optimization bug here, in
addition to the spurious warning problem.  ISO C says that if a variable is not
set in between setjmp and longjmp, then its value is preserved by the longjmp. 
GCC implements this by forcing all such variables into stack slots.  Now
suppose we have code like this, where reg100 is a user variable.
  (set reg100 ...)
  (call setjmp)
  ... (mem reg100) ...
  (set (reg101) (plus (reg100) (const_int 8)))
  ... (mem reg101) ...
  (call longjmp)
The value of reg100 should be unchanged.  On IA-64 however, gcc converts this
code to
  (set reg100 ...)
  (call setjmp)
  ... (mem (post_inc (reg100) (const_int 8)) ...
  ... (mem reg101) ...
  (call longjmp)
and now the value of reg100 is being modified, which violates the ISO C spec. 
If program execution can return to the mems after the longjmp, then the code
will not function correctly.  The wrong values will be read from memory.  This
could be a very hard bug to diagnose if it occured in real programs, and if the
longjmp call was in an exception path that rarely occured.

There is also an addition problem here that performing auto-inc optimization on
variables that are forced to stack slots is a de-optimization, as now we need
an additional memory store that we did not need before.  Without my patch, I
get
        ld8 r14 = [r15]
        ;;
        adds r14 = 8, r14
        ;;
        st8 [r15] = r14
        adds r14 = -8, r14
        ;;
        ld8 r34 = [r14], 8
        ;;
        ld8 r33 = [r14]
with my patch I get
        ld8 r15 = [r15]
        ;;
        ld8 r33 = [r15]
        adds r14 = 8, r15
        ;;
        ld8 r34 = [r14]
which is much better.  Not all cases are as lucky as this though.  In some
cases the use of auto-inc addresses would have been better.  Since this only
affects functions that call setjmp, it shouldn't be a serious problem.

One more note, I noticed that we are setting regs_live_at_setjmp, but never
clearing it.  This means it could contain more registers than it should.  But
since this only affects functions that call setjmp, I'm not going to worry
about it.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[wilson at gcc dot gnu dot org]

------- Comment #6 from wilson at gcc dot gnu dot org  2005-12-02 19:16 -------
Oops.  That patch should have a "| PROP_AUTOINC" in the new update_life_info
call.  That would explain why I was seeing some unexpected de-optimizations
with the patch.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[sje at cup dot hp dot com]

------- Comment #7 from sje at cup dot hp dot com  2009-02-20 22:36 -------
It looks like this was fixed in the 4.3 time frame so I am closing the defect.


-- 

sje at cup dot hp dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to fail|                            |4.0.2 4.0.3 4.0.4 4.1.0
                   |                            |4.1.1 4.1.2 4.2.0 4.2.1
                   |                            |4.2.2 4.2.3 4.2.4
      Known to work|                            |4.3.0 4.3.1 4.3.2 4.4.0
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[trt at acm dot org]

------- Additional Comments From trt at acm dot org  2005-06-02 18:40 -------
The current warning algorithm is too simple.  This would be better: 

   For each function that contains call(s) to setjmp(), compute:

     ref_nz The set of variables that might possibly be live
             (referenced) after a setjmp() returns a non-zero value.
     set_any The set of variables that might possibly be set
             (defined) after a call to setjmp() returns.

   Issue a warning for all variables in the intersection of the sets.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[schwab at suse dot de]

------- Additional Comments From schwab at suse dot de  2005-06-01 23:03 -------
The warning is just wrong.  There is nothing that can be clobbered by longjmp. 

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059

[Bug middle-end/21059] Bogus warning about clobbered variable

[sje at cup dot hp dot com]

------- Additional Comments From sje at cup dot hp dot com  2005-06-01 22:08 -------
It is not clear to me if this bug is about whether or not we should put out the
message or if it is about the format of the message,  I.e. that the quotes are
messed up.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sje at cup dot hp dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21059