Linaro GCC

.debug_line is wrong with -fpic

Bug #617384 reported by Yao Qi on 2010-08-13

This bug affects 1 person

	Status	Importance	Assigned to	Milestone
Linaro GCC	Fix Released	Medium	Ulrich Weigand	Linaro GCC 4.5-2011.01-0
4.4	Fix Released	Medium	Ulrich Weigand	Linaro GCC 4.4-2011.01-0
4.5	Fix Released	Medium	Ulrich Weigand	Linaro GCC 4.5-2011.01-0
Linaro GCC Tracking	Fix Released	Undecided	Unassigned	Linaro GCC Tracking 4.6.0

Bug Description

This bug is found when I was fixing linaro gdb bug LP:615989. Compile test cases in attachment, and dump .debug_line,

$ gcc -g -fpic -c test.c -o test.o
$ objdump -Wl test.o

Wrong output: (gcc-linaro-4.4-2010.08-0-armv7l)
Line Number Statements:
  Extended opcode 2: set Address to 0x0
  Advance Line by 9 to 10
  Copy
  Special opcode 49: advance Address by 6 to 0x6 and Line by 2 to 12 // <-- [1]
  Special opcode 45: advance Address by 6 to 0xc and Line by -2 to 10
  Special opcode 20: advance Address by 2 to 0xe and Line by 1 to 11
  Special opcode 62: advance Address by 8 to 0x16 and Line by 1 to 12
  Special opcode 76: advance Address by 10 to 0x20 and Line by 1 to 13
  Advance PC by 16 to 0x30
  Extended opcode 1: End of Sequence

The wrong opcode in .debug_line leads to hitting breakpoint set on function pendfunc1 on line 12 rather than line 11. As analyzed in comment #2 LP:615989, instruction on address 0x6 is generated from RTL 'pic_load_addr_thumb2', which should *not* associate with line 12.

Right output: (gcc-linaro-4.5-2010.08-1-armv7l)
Line Number Statements:
  Extended opcode 2: set Address to 0x0
  Advance Line by 9 to 10
  Copy
  Special opcode 62: advance Address by 8 to 0x8 and Line by 1 to 11
  Special opcode 62: advance Address by 8 to 0x10 and Line by 1 to 12
  Special opcode 76: advance Address by 10 to 0x1a and Line by 1 to 13
  Advance PC by 14 to 0x28
  Extended opcode 1: End of Sequence

Related branches

lp://qastaging/~uweigand/gcc-linaro/lp-617384-4.5

Merged into lp://qastaging/gcc-linaro/4.5 at revision 99449

Linaro Toolchain Developers: Pending requested 2010-12-21

lp://qastaging/~uweigand/gcc-linaro/lp-617384-4.4

Merged into lp://qastaging/gcc-linaro/4.4 at revision 93572

Linaro Toolchain Developers: Pending requested 2010-12-21

Revision history for this message

Yao Qi (yao-codesourcery) wrote on 2010-08-13:

test case Edit (163 bytes, text/plain)

Revision history for this message

Ulrich Weigand (uweigand) wrote on 2010-10-13:

The problem here is that the require_pic_register routine generates code that will be placed immediately at function entry, but does not set the location information that would cause these insns to be considered part of the prologue. Therefore, they'll just get the line number of whatever statement happened to first use the pic register.

The reason why this no longer shows up in 4.5 is that just to determine the address of a string constant, we actually do not really need the pic register anyway, and the Linaro 4.5 contains a backport of the patch that optimized code generation to avoid the pic register in this case:

2010-07-24 Sandra Loosemore <email address hidden>

Backport from mainline:

2010-04-10 Wei Guozhi <email address hidden>

        PR target/42601
        gcc/
        * config/arm/arm.c (arm_pic_static_addr): New function.
        (legitimize_pic_address): Call arm_pic_static_addr when it detects
        a static symbol.
        (arm_output_addr_const_extra): Output expression for new pattern.
        * config/arm/arm.md (UNSPEC_SYMBOL_OFFSET): New unspec symbol.

There's two options to fix this problem:

- We might want to backport the above patch to 4.4 as well (it is an additional performance enhancement anyway)

- We might want to fix the underlying problem with require_pic_register, since in more complex code the same problem could re-occur in 4.5 (or even mainline).

Revision history for this message

Ulrich Weigand (uweigand) wrote on 2010-10-13:

Fix insn locators in require_pic_register Edit (575 bytes, text/plain)

This patch implements a fix by setting the INSN_LOCATORs for all insns generated in require_pic_register to prologue_locator, so that they'll receive the same line number information as the rest of the prologue.

Revision history for this message

Michael Hope (michaelh1) wrote on 2010-10-13:

I don't think we should backport this to GCC 4.4. I consider 4.4 to be in maintenance so we should only fix regressions.

Revision history for this message

Ulrich Weigand (uweigand) wrote on 2010-10-14:

Just to clarify, there's two patches mentioned here:
(1) The performance enhancement patch by Wei Guozhi (that happened to make the GDB test case problem go away)
(2) My patch to fix the underlying bug in generating the wrong line numbers

I'd agree that we shouldn't backport (1) at this stage. However, backporting (2) does make sense to me, since this is a straight-forward low-risk change that simply fixes an obvious bug in debug info without affecting anything else.

Do you agree?

Revision history for this message

Michael Hope (michaelh1) wrote on 2010-10-14:

Backporting to 4.4 sounds fine. I'm concerned that no one will use it as new users will go for 4.5 and existing 4.4 users are in their own, regression only maintenance phase.

Michael Hope (michaelh1) on 2010-10-15

Changed in gcc-linaro:
status:	New → Confirmed
importance:	Undecided → Medium

Revision history for this message

Michael Hope (michaelh1) wrote on 2010-11-18:

Ulrich, could you get this upstream and merge as appropriate into 4.4 and 4.5?

Revision history for this message

Ulrich Weigand (uweigand) wrote on 2010-12-16:

The problem is still latent in mainline, and can be exposed again with a slightly modified test case to use an external variable instead of a constant string:

int foo (char* s);
extern char hello[];

void test (int x)
{
int y = x + 4;
foo(hello);
}

I'll post an upstream patch shortly after regression testing completes.

Revision history for this message

Ulrich Weigand (uweigand) wrote on 2010-12-20:

Upstream patch is here:
http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01570.html