Re: [cfg patch] Set locators properly

[James E Wilson <wilson at specifixinc dot com>]

Nathan Sidwell wrote:
> I'm not sure why we don't update the location for ADDR_VEC and
> ADDR_DIFF_VEC insns, but do set their locations, this seems inconsistent
> and I've changed that to make it consistent -- if this is incorrect
> could some one explain so I can add a comment there.

I suspect the issue here is that an ADDR_VEC/ADDR_DIFF_VEC always 
follows an unconditional branch, and should have the same block/line 
info as the branch.  But since it is after an unconditional branch, it 
ends up being considered between blocks.  If you try to record 
block/line info for an ADDR_VEC/ADDR_DIFF_VEC, you might end up with 
some confused info.  Also, you might accidentally end up with a line 
number note in between the ADDR_VEC/ADDR_DIFF_VEC and the preceeding 
branch.  However, we still should be setting the location info, setting 
it to be the same as the previous branch preferrably.

This would explain why the current code was written that way, though I 
am not sure that it proves the code is necessary.  It might be relying 
on old assumptions.

Actually, looking at this a bit more, the jump tables emitted by 
ADDR_VEC/ADDR_DIFF_VEC may go into different sections, e.g. rodata.  If 
we happened to emit a line number note into rodata that would be very 
suprising, and possibly very bad.  If we guarantee that the 
ADDR_VEC/ADDR_DIFF_VEC always has the same line/block/file info as the 
preceeding branch, then we guarantee that we won't be accidentally 
emitting a line number into the rodata section.  However, I see that in 
final, we always early out for ADDR_VEC/ADDR_DIFF_VEC before hitting the 
notice_source_line call, thus it doesn't matter here whether they have 
INSN_LOCATION set or not.  Otherwise, you would get an ICE here with 
your patch.

I guess the way you have it is OK.  If this doesn't work, then 
definitely we can comment why.

> built & tested on i686-pc-linux-gnu, ok?

This is no good if you are testing a debug patch.  A gcc bootstrap does 
nothing to test the correctness of the debug info.  It only proves that 
it won't core dump.  To test this properly, you need to run the gdb 
testsuite with patched and unpatched gcc's, and verify that there are no 
regressions.

Jan did this when he originally wrote the code.  See
     http://gcc.gnu.org/ml/gcc-patches/2003-06/msg00529.html

This is OK if it passes the gdb testsuite.
--
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com