[DWARF] Incorrect prologue end line record.

[CarlosAlbertoEnciso]

Summary

The prologue-end line record must be emitted after the last instruction that is part of the function frame setup code and before the instruction that marks the beginning of the function body.

For the below test:

1 extern int get_arg();
2 extern void func(int x);
3
4 int main()
5 {
6 int a;
7 func(get_arg());
8 }
9

The prologue-end line record is emitted with an incorrect associated address, which causes a debugger to show the beginning of function body to be inside the prologue.

This can be seen in the following trimmed assembler output:

main:
  ...
  .loc	1 7 0 prologue_end
  pushq	%rax
.Lcfi0:
  .cfi_def_cfa_offset 16
  callq	_Z7get_argv
  ...
  retq

The instruction 'pushq %rax' is part of the frame setup code.

The correct location for the prologue-end line information is just before the call to '_Z7get_argv', as illustrated in the following trimmed assembler output:

main:
  ...
  pushq	%rax
.Lcfi0:
  .cfi_def_cfa_offset 16
  .loc	1 7 0 prologue_end
  callq	_Z7get_argv
  ...
  retq

Notes:

• This patch does not change the LLDB test cases that were failing with a previous patch.

• As per reference, there are 2 previous related revisions:

https://reviews.llvm.org/D37625

Closed by commit rL313047
Reverted in rL313057 as it was causing buildbot failure (lldb inline stepping tests).

https://reviews.llvm.org/D39283

Patch with the required changes to the failing tests.
After some discussions with Tamas, it was clear that changing the test cases was not an optimal solution.
This revision should be abandoned.

[aprantl]

Just a general comment: It is easier to review patches with more context. I usually upload the output of git diff HEAD~1 -U9999.

[aprantl]

Is this an X86-only problem or is this a fix for a general problem on X86 only?

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1172 ↗

(On Diff #128717)

Can you explain why this is necessary / convince me that this is also true for shrink-wrapped code? I could imagine that for shrink-wrapped code, where the frame setup is sunk to the latest possible point it would still be useful to know accurate line numbers for code before the frame setup.

[CarlosAlbertoEnciso]

In D41762#968441, @aprantl wrote:

Just a general comment: It is easier to review patches with more context. I usually upload the output of git diff HEAD~1 -U9999.

Thanks very much for your suggestion. Would you like for me to update te patch?

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1172 ↗

(On Diff #128717)

I will have a look at it. Thanks.

[CarlosAlbertoEnciso]

Due to the uploaded patch, the following test case fails:

llvm/test/DebugInfo/COFF/types-array.ll

Before the patch:

# %bb.0:                                # %entry
	pushl	%ebp
	.cv_fpo_pushreg	%ebp
	movl	%esp, %ebp
	.cv_fpo_setframe	%ebp
Ltmp0:
	subl	$20, %esp
	.cv_loc	0 1 3 7                 # t.cpp:3:7
	movl	"?a@?1??f@@YAXXZ@3PAHA"+16, %eax

After the patch:

# %bb.0:                                # %entry
	pushl	%ebp
	.cv_fpo_pushreg	%ebp
	movl	%esp, %ebp
	.cv_fpo_setframe	%ebp
	subl	$20, %esp
Ltmp0:
	.cv_loc	0 1 3 7                 # t.cpp:3:7
	movl	"?a@?1??f@@YAXXZ@3PAHA"+16, %eax

The reason for the failure is because now the frame setup code is correct. The instruction

subl	$20, %esp

now is part of the prologue. Before the patch it was the first instruction of the function body.

The new patch will include the modified test case.

[CarlosAlbertoEnciso]

As the patch is intended to fix the invalid prologue-end records, the added check is looking for instructions associated with the frame setup: MachineInstr::FrameSetup flag.

When generating Debug Location (line records) at the start of an instruction, that check stops emitting a line record for instructions with no correspondence to user code, otherwise a normal line record will be emitted.

In the case of 'shrink-wrapped code', the check is valid, as instructions before the frame setup will have correspondence to user code, hence accurate line records will be emitted.

Essentially line records for instructions that are not part of the epilogue/prologue are not affected by this change.

[CarlosAlbertoEnciso]

Apologies as the original test case for the patch was incorrect, as I uploaded an old test case.

[CarlosAlbertoEnciso]

This update contains:

Fix for a failed test case: llvm/test/DebugInfo/COFF/types-array.ll
Correct test case for the patch.

[CarlosAlbertoEnciso]

Ping.

[aprantl]

In the case of 'shrink-wrapped code', the check is valid, as instructions before the frame setup will have correspondence to user code, hence accurate line records will be emitted.

This explanation does not validate against my mental model of shrink-wrapping. It's possible that my mental model is incorrect, but my understanding is that shrink-wrapping defers the frame setup until the first instruction that needs access to something in the frame. But that does not imply that there is no correspondence to user code before the frame setup is complete.

[qcolombet]

But that does not imply that there is no correspondence to user code before the frame setup is complete.

Adrian is correct, there could be a correspondence to user code before the frame setup is complete.

[CarlosAlbertoEnciso]

There could be a correspondence to user code before the frame setup is complete.

Hi Adrian & Quentin,

Thanks very much for your comments. Now I can see your point of concern.

I will try to create a test case using the ones committed with the changes to support 'shrink-wrapped code'.

[CarlosAlbertoEnciso]

Hi Adrian & Quentin,

For the below test

extern int doSomething(int a,int *b);

int foo(int a,int b)
{
  int tmp4 = a;
  if (a < b) {
    int tmp = a;
    tmp4 = doSomething(0,&tmp);
  }

  return tmp4;
}

that generates similar IR as the first test case from

llvm/test/CodeGen/X86/x86-shrink-wrapping.ll

and generating only debug line table

define i32 @_Z3fooii(i32 %a, i32 %b) !dbg !7 {
entry:
  %tmp = alloca i32, align 4
  %cmp = icmp slt i32 %a, %b, !dbg !9
  br i1 %cmp, label %if.then, label %if.end, !dbg !10

if.then:                                          ; preds = %entry
  store i32 %a, i32* %tmp, align 4, !dbg !11
  %call = call i32 @_Z11doSomethingiPi(i32 0, i32* %tmp), !dbg !12
  br label %if.end, !dbg !13

if.end:                                           ; preds = %if.then, %entry
  %tmp4.0 = phi i32 [ %call, %if.then ], [ %a, %entry ]
  ret i32 %tmp4.0, !dbg !14
}

declare i32 @_Z11doSomethingiPi(i32, i32*)
...

The line records generated with '-enable-shrink-wrap=false/true' are the same.

[CarlosAlbertoEnciso]

Hi Adrian & Quentin,

For the below test (which combines the original test case used to reproduce the invalid prologue end issue)

extern int doSomething(int a,int *b);
extern int getArg();

int foo(int a,int b)
{
  int tmp4 = a;
  if (a < b) {
    int tmp = a;
    tmp4 = doSomething(getArg(),&tmp);
  }

  return tmp4;
}

that generates similar IR as the first test case from

llvm/test/CodeGen/X86/x86-shrink-wrapping.ll

and generating only debug line table

define i32 @_Z3fooii(i32 %a, i32 %b) !dbg !7 {
entry:
  %tmp = alloca i32, align 4
  %cmp = icmp slt i32 %a, %b, !dbg !9
  br i1 %cmp, label %if.then, label %if.end, !dbg !10

if.then:                                          ; preds = %entry
  store i32 %a, i32* %tmp, align 4, !dbg !11
  %call = call i32 @_Z6getArgv(), !dbg !12
  %call1 = call i32 @_Z11doSomethingiPi(i32 %call, i32* %tmp), !dbg !13
  br label %if.end, !dbg !14

if.end:                                           ; preds = %if.then, %entry
  %tmp4.0 = phi i32 [ %call1, %if.then ], [ %a, %entry ]
  ret i32 %tmp4.0, !dbg !15
}

declare i32 @_Z11doSomethingiPi(i32, i32*)
declare i32 @_Z6getArgv()

The line records generated with '-enable-shrink-wrap=false/true' are the same.

[CarlosAlbertoEnciso]

Hi Adrian & Quentin,

For the below test (which combines the original test case used to reproduce the invalid prologue end issue)

extern int doSomething(int a,int *b);
extern int getArg();

int foo(int a,int b)
{
  int tmp4 = a;
  if (a < b) {
    int tmp = a;
    tmp4 = doSomething(getArg(),&tmp);
  }

  return tmp4;
}

that generates similar IR as the first test case from

llvm/test/CodeGen/X86/x86-shrink-wrapping.ll

and generating only debug line table

define i32 @_Z3fooii(i32 %a, i32 %b) !dbg !7 {
entry:
  %tmp = alloca i32, align 4
  %cmp = icmp slt i32 %a, %b, !dbg !9
  br i1 %cmp, label %if.then, label %if.end, !dbg !10

if.then:                                          ; preds = %entry
  store i32 %a, i32* %tmp, align 4, !dbg !11
  %call = call i32 @_Z6getArgv(), !dbg !12
  %call1 = call i32 @_Z11doSomethingiPi(i32 %call, i32* %tmp), !dbg !13
  br label %if.end, !dbg !14

if.end:                                           ; preds = %if.then, %entry
  %tmp4.0 = phi i32 [ %call1, %if.then ], [ %a, %entry ]
  ret i32 %tmp4.0, !dbg !15
}

declare i32 @_Z11doSomethingiPi(i32, i32*)
declare i32 @_Z6getArgv()

The line records generated with '-enable-shrink-wrap=false/true' are the same.

[CarlosAlbertoEnciso]

Hi Quentin,

If you have a more complex test case (from your work on shrink-wrapping code', that I can use to test the changes when the '-enable-shrink-wrap' option is enable, it would be a great help.

Thanks
-Carlos

[CarlosAlbertoEnciso]

Hi Adrian & Quentin,

These are the line records output (llvm-dwarfdump):

First test case: -enable-shrink-wrap=false

Address            Line   Column File   ISA Discriminator Flags
------------------ ------ ------ ------ --- ------------- -------------
0x0000000000000000      4      0      1   0             0  is_stmt
0x0000000000000003      6      9      1   0             0  is_stmt prologue_end
0x0000000000000005      6      7      1   0             0 
0x0000000000000007      7      9      1   0             0  is_stmt
0x0000000000000010      8     12      1   0             0  is_stmt
0x0000000000000017     11      3      1   0             0  is_stmt
0x0000000000000019     11      3      1   0             0  is_stmt end_sequence

First test case: -enable-shrink-wrap=true

Address            Line   Column File   ISA Discriminator Flags
------------------ ------ ------ ------ --- ------------- -------------
0x0000000000000000      4      0      1   0             0  is_stmt
0x0000000000000002      6      9      1   0             0  is_stmt prologue_end
0x0000000000000004      6      7      1   0             0 
0x0000000000000007      7      9      1   0             0  is_stmt
0x0000000000000010      8     12      1   0             0  is_stmt
0x000000000000001b     11      3      1   0             0  is_stmt
0x000000000000001c     11      3      1   0             0  is_stmt end_sequence

Second test case: -enable-shrink-wrap=false

Address            Line   Column File   ISA Discriminator Flags
------------------ ------ ------ ------ --- ------------- -------------
0x0000000000000000      5      0      1   0             0  is_stmt
0x0000000000000003      7      9      1   0             0  is_stmt prologue_end
0x0000000000000005      7      7      1   0             0 
0x0000000000000007      8      9      1   0             0  is_stmt
0x000000000000000b      9     24      1   0             0  is_stmt
0x0000000000000015      9     12      1   0             0 
0x000000000000001c     12      3      1   0             0  is_stmt
0x000000000000001e     12      3      1   0             0  is_stmt end_sequence

Second test case: -enable-shrink-wrap=true

Address            Line   Column File   ISA Discriminator Flags
------------------ ------ ------ ------ --- ------------- -------------
0x0000000000000000      5      0      1   0             0  is_stmt
0x0000000000000002      7      9      1   0             0  is_stmt prologue_end
0x0000000000000004      7      7      1   0             0 
0x0000000000000007      8      9      1   0             0  is_stmt
0x000000000000000b      9     24      1   0             0  is_stmt
0x0000000000000015      9     12      1   0             0 
0x0000000000000020     12      3      1   0             0  is_stmt
0x0000000000000021     12      3      1   0             0  is_stmt end_sequence

Thanks

[qcolombet]

Hi Carlos,

Does shrink-wrapping change the assembly in your examples?
If you want to have more examples, you can take a look at test/CodeGen/X86/x86-shrink-wrapping.ll and/or run the llvm test-suite with and without the shrink-wrapping option and look at the diffs.

Cheers,
-Quentin

[CarlosAlbertoEnciso]

Hi Quentin,

Does shrink-wrapping change the assembly in your examples?

Yes; it does. I have copied edited assembly (removed some temporal labels and debug info).

[CarlosAlbertoEnciso]

First test case:
-enable-shrink-wrap=false

# %bb.0:                        # %entry
	pushq	%rax
	movl	%edi, %eax
	cmpl	%esi, %eax
	jge	.LBB0_2
# %bb.1:                        # %if.then
	movl	%eax, 4(%rsp)
	leaq	4(%rsp), %rsi
	xorl	%edi, %edi
	callq	_Z11doSomethingiPi
.LBB0_2:                        # %if.end
	popq	%rcx
	retq

-enable-shrink-wrap=true

# %bb.0:                        # %entry
	movl	%edi, %eax
	cmpl	%esi, %eax
	jge	.LBB0_2
# %bb.1:                        # %if.then
	pushq	%rax
	movl	%eax, 4(%rsp)
	leaq	4(%rsp), %rsi
	xorl	%edi, %edi
	callq	_Z11doSomethingiPi
	addq	$8, %rsp
.LBB0_2:                        # %if.end
	retq

The following instructions have changed and moved:

'pushq	%rax'             move from %bb.0 to %bb.1
'popq	%rcx'             changed into  'addq	$8, %rsp',

[CarlosAlbertoEnciso]

Second test case:
-enable-shrink-wrap=false

# %bb.0:                                # %entry
pushq	%rax
	movl	%edi, %eax
	cmpl	%esi, %eax
	jge	.LBB0_2
# %bb.1:                                # %if.then
	movl	%eax, 4(%rsp)
	callq	_Z6getArgv
	leaq	4(%rsp), %rsi
	movl	%eax, %edi
	callq	_Z11doSomethingiPi
.LBB0_2:                                # %if.end
	popq	%rcx
	retq

-enable-shrink-wrap=true

# %bb.0:                                # %entry
	movl	%edi, %eax
	cmpl	%esi, %eax
	jge	.LBB0_2
# %bb.1:                                # %if.then
	pushq	%rax
	movl	%eax, 4(%rsp)
	callq	_Z6getArgv
	leaq	4(%rsp), %rsi
	movl	%eax, %edi
	callq	_Z11doSomethingiPi
	addq	$8, %rsp
.LBB0_2:                                # %if.end
	retq

The following instructions have changed and moved:

'pushq	%rax'             moved from %bb.0 to %bb.1
'popq	%rcx'             changed into  'addq	$8, %rsp'

[CarlosAlbertoEnciso]

Hi Quentin,

If you want to have more examples, you can take a look at test/CodeGen/X86/x86-shrink-wrapping.ll and/or run the llvm test-suite with and without the shrink-wrapping option and look at the diffs.

I will take a further look at the referenced test case and run the llvm test-suite. I will let you know the results.

Thanks,
Carlos

[CarlosAlbertoEnciso]

Hi Quentin,

run the llvm test-suite with and without the shrink-wrapping option and look at the diffs.

I forced the '-enable-shring-wrap' to be true/false and run the 'check-llvm' and 'check-lldb' tests.

The output from 'check-lldb' are the same.

There are some differences in the output from 'check-llvm' and corresponds to failures due to test cases expecting a specific value for the flag.

I am setting up the llvm test-suite, so I can run it. I would let you know the results.

Thanks,
Carlos

[CarlosAlbertoEnciso]

Hi Quentin,

run the llvm test-suite with and without the shrink-wrapping option and look at the diffs.

I managed to setup the llvm test-suite and the results are the same. There is only 1 failure regardless of the option setting.

Failing Tests (1):
  test-suite :: SingleSource/UnitTests/ms_struct_pack_layout.test

The same failure is present without my changes.

Thanks,
Carlos

[CarlosAlbertoEnciso]

Ping.

Thanks,
Carlos

[qcolombet]

For the review itself, I leave this to @aprantl. I was just providing insights on the shrink-wrapping question :).

[CarlosAlbertoEnciso]

In D41762#1002090, @qcolombet wrote:

For the review itself, I leave this to @aprantl. I was just providing insights on the shrink-wrapping question :).

Hi Quentin,

Thanks very much for your help and feedback.

Carlos

[CarlosAlbertoEnciso]

Hi Adrian,

In D41762#1002090, @qcolombet wrote:

For the review itself, I leave this to @aprantl. I was just providing insights on the shrink-wrapping question :).

Do you have any additional concerns or comments in relation to the shrink-wrapping, that I have not covered with Quentin?

Thanks,
Carlos

[aprantl]

Thanks for posting the examples and for doing due diligence on shrink wrapping! IIUC, enabling/disabling shrink-wrapping does not move the prologue_end, so this LGTM.

[thegameg]

In D41762#1006344, @aprantl wrote:

IIUC, enabling/disabling shrink-wrapping does not move the prologue_end, so this LGTM.

If I understand the code correctly, in case of shrink-wrapping kicking in, prologue_end will be at the beginning of the function, while without shrink-wrapping, it will be at the end of the prologue. I believe that's correct, right?

$ cat foo.c
void foo(int *);

int main(int argc, char const *argv[])
{
  if (argc > 109) {
    int j = 30;
    foo(&j);
    return j;
  }
  return 0;
}

$ clang -O1 -S -g -o - foo.c
_main:                                  ## @main
        [...]
## %bb.0:                               ## %entry
	xorl	%eax, %eax
Ltmp0:
	.loc	1 5 12 prologue_end     ## foo.c:5:12
	cmpl	$110, %edi
	[...]
	jl	LBB0_2
## %bb.1:                               ## %if.then
        [...]
	pushq	%rbp
	[...]
	popq	%rbp
LBB0_2:                                 ## %return
	[...]
	retq

$ clang -O1 -S -g -o - foo.c -mllvm -enable-shrink-wrap=false
_main:                                  ## @main
	[...]
## %bb.0:                               ## %entry
	pushq	%rbp
        [.cfi...]
	movq	%rsp, %rbp
	subq	$16, %rsp
	[...]
	xorl	%eax, %eax
Ltmp0:
	.loc	1 5 12 prologue_end     ## foo.c:5:12
	cmpl	$110, %edi
	[...]
	jl	LBB0_2
Ltmp2:
## %bb.1:                               ## %if.then
	[...]
LBB0_2:                                 ## %return
	[...]
	popq	%rbp
	retq

[CarlosAlbertoEnciso]

In D41762#1006539, @thegameg wrote:

In D41762#1006344, @aprantl wrote:

IIUC, enabling/disabling shrink-wrapping does not move the prologue_end, so this LGTM.

If I understand the code correctly, in case of shrink-wrapping kicking in, prologue_end will be at the beginning of the function, while without shrink-wrapping, it will be at the end of the prologue. I believe that's correct, right?

Hi,

I tried your given test case with the same command line options and this is the assembler generated for both cases:

clang -O1 -S -g -o - foo.c

main:                           # @main
  [...]
# %bb.0:                        # %entry
  xorl	%eax, %eax
.Ltmp0:
  .loc	1 5 12 prologue_end     # foo.c:5:12
  cmpl	$110, %edi
.Ltmp1:
  .loc	1 5 7 is_stmt 0         # foo.c:5:7
  jl	.LBB0_2
.Ltmp2:
# %bb.1:                        # %if.then
  pushq	%rax
  .cfi_def_cfa_offset 16
.Ltmp3:
  .loc	1 6 9 is_stmt 1         # foo.c:6:9
  movl	$30, 4(%rsp)
  leaq	4(%rsp), %rdi
.Ltmp4:
  .loc	1 7 5                   # foo.c:7:5
  callq	foo
.Ltmp5:
  .loc	1 8 12                  # foo.c:8:12
  movl	4(%rsp), %eax
.Ltmp6:
  .loc	1 0 12 is_stmt 0        # foo.c:0:12
  addq	$8, %rsp
.Ltmp7:
.LBB0_2:                        # %return
  .loc	1 11 1 is_stmt 1        # foo.c:11:1
  retq

clang -O1 -S -g -o - foo.c -mllvm -enable-shrink-wrap=false

main:                           # @main
  [...]
# %bb.0:                        # %entry
  pushq	%rax
  .cfi_def_cfa_offset 16
  xorl	%eax, %eax
.Ltmp0:
  .loc	1 5 12 prologue_end     # foo.c:5:12
  cmpl	$110, %edi
.Ltmp1:
  .loc	1 5 7 is_stmt 0         # foo.c:5:7
  jl	.LBB0_2
.Ltmp2:
# %bb.1:                        # %if.then
  .loc	1 6 9 is_stmt 1         # foo.c:6:9
  movl	$30, 4(%rsp)
  leaq	4(%rsp), %rdi
.Ltmp3:
  .loc	1 7 5                   # foo.c:7:5
  callq	foo
.Ltmp4:
  .loc	1 8 12                  # foo.c:8:12
  movl	4(%rsp), %eax
.Ltmp5:
.LBB0_2:                        # %return
  .loc	1 11 1                  # foo.c:11:1
  popq	%rcx
  retq

I noticed that the code generation does not look similar to your posting specially for the case when -enable-shrink-wrap=false.

May be we are using different compilers. My version is revision 325102 plus the submitted changes for the review.

Thanks,
Carlos

[CarlosAlbertoEnciso]

In D41762#1006344, @aprantl wrote:

Thanks for posting the examples and for doing due diligence on shrink wrapping! IIUC, enabling/disabling shrink-wrapping does not move the prologue_end, so this LGTM.

Hi Adrian,

You are correct. Enabling/disabling shrink-wrapping does not move the prologue_end debug records.

Thanks very much for your suggestions, feedback and review.

Carlos

[thegameg]

In D41762#1007199, @CarlosAlbertoEnciso wrote:

In D41762#1006539, @thegameg wrote:

In D41762#1006344, @aprantl wrote:

IIUC, enabling/disabling shrink-wrapping does not move the prologue_end, so this LGTM.

If I understand the code correctly, in case of shrink-wrapping kicking in, prologue_end will be at the beginning of the function, while without shrink-wrapping, it will be at the end of the prologue. I believe that's correct, right?

Hi,

I tried your given test case with the same command line options and this is the assembler generated for both cases:

The difference here is the push %rax.

clang -O1 -S -g -o - foo.c

In this case, the push (the prologue) is in bb.1, after the cmp.

clang -O1 -S -g -o - foo.c -mllvm -enable-shrink-wrap=false

In this case, the push (the prologue) is in bb.0, before the cmp.

I noticed that the code generation does not look similar to your posting specially for the case when -enable-shrink-wrap=false.

Yes, sorry, you should be able to achieve the same result with -fno-omit-frame-pointer in both commands.

May be we are using different compilers. My version is revision 325102 plus the submitted changes for the review.

My bad, I should have provided the correct triple: -target x86_64-apple-darwin.

[CarlosAlbertoEnciso]

In D41762#1007447, @thegameg wrote:

In D41762#1007199, @CarlosAlbertoEnciso wrote:

I noticed that the code generation does not look similar to your posting specially for the case when -enable-shrink-wrap=false.

Yes, sorry, you should be able to achieve the same result with -fno-omit-frame-pointer in both commands.
My bad, I should have provided the correct triple: -target x86_64-apple-darwin.

Now using those extra options, I am be able to achieve the same results.

Thanks

[CarlosAlbertoEnciso]

In D41762#1007447, @thegameg wrote:

The difference here is the push %rax.

clang -O1 -S -g -o - foo.c

In this case, the push (the prologue) is in bb.1, after the cmp.

clang -O1 -S -g -o - foo.c -mllvm -enable-shrink-wrap=false

In this case, the push (the prologue) is in bb.0, before the cmp.

Hi,

I think the following links describe your findings.

https://reviews.llvm.org/D41762#990580
https://reviews.llvm.org/D41762#990589
https://reviews.llvm.org/D41762#990611

The assembler lines change, but the debug line records remain the same.

Thanks,
Carlos

[thegameg]

Thanks @CarlosAlbertoEnciso, I finally understood the original concern about shrink-wrapping. LGTM.

[CarlosAlbertoEnciso]

In D41762#1007640, @thegameg wrote:

Thanks @CarlosAlbertoEnciso, I finally understood the original concern about shrink-wrapping. LGTM.

Thanks @thegameg for your review.

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

void DwarfDebug::beginInstruction(const MachineInstr *MI) {
   DebugHandlerBase::beginInstruction(MI);
   assert(CurMI);
 
   const auto *SP = MI->getMF()->getFunction().getSubprogram();
   if (!SP || SP->getUnit()->getEmissionKind() == DICompileUnit::NoDebug)
     return;
 
   // Check if source location changes, but ignore DBG_VALUE and CFI locations.
-  if (MI->isMetaInstruction())
+  // If the instruction is part of the function frame setup code, do not emit
+  // any line record, as there is no correspondence with any user code.
+  if (MI->isMetaInstruction() || MI->getFlag(MachineInstr::FrameSetup))
     return;
   const DebugLoc &DL = MI->getDebugLoc();
   // When we emit a line-0 record, we don't update PrevInstLoc; so look at
   // the last line number actually emitted, to see if it was line 0.
   unsigned LastAsmLine =
       Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine();
 
   if (DL == PrevInstLoc) {

llvm/trunk/lib/Target/X86/X86ExpandPseudo.cpp

case X86::TCRETURNmi64: {
 
     if (Opcode == X86::TCRETURNdicc || Opcode == X86::TCRETURNdi64cc) {
       assert(Offset == 0 && "Conditional tail call cannot adjust the stack.");
     }
 
     if (Offset) {
       // Check for possible merge with preceding ADD instruction.
       Offset += X86FL->mergeSPUpdates(MBB, MBBI, true);
-      X86FL->emitSPUpdate(MBB, MBBI, Offset, /*InEpilogue=*/true);
+      X86FL->emitSPUpdate(MBB, MBBI, DL, Offset, /*InEpilogue=*/true);
     }
 
     // Jump to label or value in register.
     bool IsWin64 = STI->isTargetWin64();
     if (Opcode == X86::TCRETURNdi || Opcode == X86::TCRETURNdicc ||
         Opcode == X86::TCRETURNdi64 || Opcode == X86::TCRETURNdi64cc) {
       unsigned Op;
       switch (Opcode) {
BuildMI(MBB, MBBI, DL,
             TII->get(Uses64BitFramePtr ? X86::MOV64rr : X86::MOV32rr), StackPtr)
         .addReg(DestAddr.getReg());
     // The EH_RETURN pseudo is really removed during the MC Lowering.
     return true;
   }
   case X86::IRET: {
     // Adjust stack to erase error code
     int64_t StackAdj = MBBI->getOperand(0).getImm();
-    X86FL->emitSPUpdate(MBB, MBBI, StackAdj, true);
+    X86FL->emitSPUpdate(MBB, MBBI, DL, StackAdj, true);
     // Replace pseudo with machine iret
     BuildMI(MBB, MBBI, DL,
             TII->get(STI->is64Bit() ? X86::IRET64 : X86::IRET32));
     MBB.erase(MBBI);
     return true;
   }
   case X86::RET: {
     // Adjust stack to erase error code
     int64_t StackAdj = MBBI->getOperand(0).getImm();
     MachineInstrBuilder MIB;
     if (StackAdj == 0) {
       MIB = BuildMI(MBB, MBBI, DL,
                     TII->get(STI->is64Bit() ? X86::RETQ : X86::RETL));
     } else if (isUInt<16>(StackAdj)) {
       MIB = BuildMI(MBB, MBBI, DL,
                     TII->get(STI->is64Bit() ? X86::RETIQ : X86::RETIL))
                 .addImm(StackAdj);
     } else {
       assert(!STI->is64Bit() &&
              "shouldn't need to do this for x86_64 targets!");
       // A ret can only handle immediates as big as 2**16-1.  If we need to pop
       // off bytes before the return address, we must do it manually.
       BuildMI(MBB, MBBI, DL, TII->get(X86::POP32r)).addReg(X86::ECX, RegState::Define);
-      X86FL->emitSPUpdate(MBB, MBBI, StackAdj, /*InEpilogue=*/true);
+      X86FL->emitSPUpdate(MBB, MBBI, DL, StackAdj, /*InEpilogue=*/true);
       BuildMI(MBB, MBBI, DL, TII->get(X86::PUSH32r)).addReg(X86::ECX);
       MIB = BuildMI(MBB, MBBI, DL, TII->get(X86::RETL));
     }
     for (unsigned I = 1, E = MBBI->getNumOperands(); I != E; ++I)
       MIB.add(MBBI->getOperand(I));
     MBB.erase(MBBI);
     return true;
   }

llvm/trunk/lib/Target/X86/X86FrameLowering.h

public:
   /// stack adjustment is returned as a positive value for ADD/LEA and
   /// a negative for SUB.
   int mergeSPUpdates(MachineBasicBlock &MBB, MachineBasicBlock::iterator &MBBI,
                      bool doMergeWithPrevious) const;
 
   /// Emit a series of instructions to increment / decrement the stack
   /// pointer by a constant value.
   void emitSPUpdate(MachineBasicBlock &MBB, MachineBasicBlock::iterator &MBBI,
-                    int64_t NumBytes, bool InEpilogue) const;
+                    const DebugLoc &DL, int64_t NumBytes, bool InEpilogue) const;
 
   /// Check that LEA can be used on SP in an epilogue sequence for \p MF.
   bool canUseLEAForSPInEpilogue(const MachineFunction &MF) const;
 
   /// Check whether or not the given \p MBB can be used as a prologue
   /// for the target.
   /// The prologue will be inserted first in this basic block.
   /// This method is used by the shrink-wrapping pass to decide if

llvm/trunk/lib/Target/X86/X86FrameLowering.cpp

flagsNeedToBePreservedBeforeTheTerminators(const MachineBasicBlock &MBB) {
 
   return false;
 }
 
 /// emitSPUpdate - Emit a series of instructions to increment / decrement the
 /// stack pointer by a constant value.
 void X86FrameLowering::emitSPUpdate(MachineBasicBlock &MBB,
                                     MachineBasicBlock::iterator &MBBI,
+                                    const DebugLoc &DL,
                                     int64_t NumBytes, bool InEpilogue) const {
   bool isSub = NumBytes < 0;
   uint64_t Offset = isSub ? -NumBytes : NumBytes;
   MachineInstr::MIFlag Flag =
       isSub ? MachineInstr::FrameSetup : MachineInstr::FrameDestroy;
 
   uint64_t Chunk = (1LL << 31) - 1;
-  DebugLoc DL = MBB.findDebugLoc(MBBI);
 
   if (Offset > Chunk) {
     // Rather than emit a long series of instructions for large offsets,
     // load the offset into a register and do one sub/add
     unsigned Reg = 0;
     unsigned Rax = (unsigned)(Is64Bit ? X86::RAX : X86::EAX);
 
     if (isSub && !isEAXLiveIn(MBB))
void X86FrameLowering::emitPrologue(MachineFunction &MF,
 
   // Re-align the stack on 64-bit if the x86-interrupt calling convention is
   // used and an error code was pushed, since the x86-64 ABI requires a 16-byte
   // stack alignment.
   if (Fn.getCallingConv() == CallingConv::X86_INTR && Is64Bit &&
       Fn.arg_size() == 2) {
     StackSize += 8;
     MFI.setStackSize(StackSize);
-    emitSPUpdate(MBB, MBBI, -8, /*InEpilogue=*/false);
+    emitSPUpdate(MBB, MBBI, DL, -8, /*InEpilogue=*/false);
   }
 
   // If this is x86-64 and the Red Zone is not disabled, if we are a leaf
   // function, and use up to 128 bytes of stack space, don't have a frame
   // pointer, calls, or dynamic alloca then we do not need to adjust the
   // stack pointer (we fit in the Red Zone). We also check that we don't
   // push and pop from the stack.
   if (Is64Bit && !Fn.hasFnAttribute(Attribute::NoRedZone) &&
if (isEAXAlive) {
       // Restore EAX
       MachineInstr *MI =
           addRegOffset(BuildMI(MF, DL, TII.get(X86::MOV32rm), X86::EAX),
                        StackPtr, false, NumBytes - 4);
       MI->setFlag(MachineInstr::FrameSetup);
       MBB.insert(MBBI, MI);
     }
   } else if (NumBytes) {
-    emitSPUpdate(MBB, MBBI, -(int64_t)NumBytes, /*InEpilogue=*/false);
+    emitSPUpdate(MBB, MBBI, DL, -(int64_t)NumBytes, /*InEpilogue=*/false);
   }
 
   if (NeedsWinCFI && NumBytes) {
     HasWinCFI = true;
     BuildMI(MBB, MBBI, DL, TII.get(X86::SEH_StackAlloc))
         .addImm(NumBytes)
         .setMIFlag(MachineInstr::FrameSetup);
   }
if ((TRI->needsStackRealignment(MF) || MFI.hasVarSizedObjects()) &&
     } else {
       unsigned Opc = (Uses64BitFramePtr ? X86::MOV64rr : X86::MOV32rr);
       BuildMI(MBB, MBBI, DL, TII.get(Opc), StackPtr)
         .addReg(FramePtr);
       --MBBI;
     }
   } else if (NumBytes) {
     // Adjust stack pointer back: ESP += numbytes.
-    emitSPUpdate(MBB, MBBI, NumBytes, /*InEpilogue=*/true);
+    emitSPUpdate(MBB, MBBI, DL, NumBytes, /*InEpilogue=*/true);
     --MBBI;
   }
 
   // Windows unwinder will not invoke function's exception handler if IP is
   // either in prologue or in epilogue.  This behavior causes a problem when a
   // call immediately precedes an epilogue, because the return address points
   // into the epilogue.  To cope with that, we insert an epilogue marker here,
   // then replace it with a 'nop' if it ends up immediately after a CALL in the
   // final emitted code.
   if (NeedsWin64CFI && MF.hasWinCFI())
     BuildMI(MBB, MBBI, DL, TII.get(X86::SEH_Epilogue));
 
   if (Terminator == MBB.end() || !isTailCallOpcode(Terminator->getOpcode())) {
     // Add the return addr area delta back since we are not tail calling.
     int Offset = -1 * X86FI->getTCReturnAddrDelta();
     assert(Offset >= 0 && "TCDelta should never be positive");
     if (Offset) {
       // Check for possible merge with preceding ADD instruction.
       Offset += mergeSPUpdates(MBB, Terminator, true);
-      emitSPUpdate(MBB, Terminator, Offset, /*InEpilogue=*/true);
+      emitSPUpdate(MBB, Terminator, DL, Offset, /*InEpilogue=*/true);
     }
   }
 }
 
 int X86FrameLowering::getFrameIndexReference(const MachineFunction &MF, int FI,
                                              unsigned &FrameReg) const {
   const MachineFrameInfo &MFI = MF.getFrameInfo();
 

llvm/trunk/test/DebugInfo/COFF/types-array.ll

 ; CHECK:       VarName: a
 ; CHECK:     }
 ; CHECK:     DefRangeRegisterRelSym {
 ; CHECK:       BaseRegister: EBP (0x16)
 ; CHECK:       HasSpilledUDTMember: No
 ; CHECK:       OffsetInParent: 0
 ; CHECK:       BasePointerOffset: -20
 ; CHECK:       LocalVariableAddrRange {
-; CHECK:         OffsetStart: .text+0x3
+; CHECK:         OffsetStart: .text+0x6
 ; CHECK:         ISectStart: 0x0
-; CHECK:         Range: 0x36
+; CHECK:         Range: 0x33
 ; CHECK:       }
 ; CHECK:     }
 ; CHECK:     ProcEnd {
 ; CHECK:     }
 ; CHECK:   ]
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"

llvm/trunk/test/DebugInfo/X86/invalid-prologue-end.ll

+; RUN: llc -mtriple=x86_64-linux-gnu -filetype=asm %s -o - | FileCheck %s
+
+; The prologue-end line record must be emitted after the last instruction that
+; is part of the function frame setup code and before the instruction that marks
+; the beginning of the function body.
+;
+; For the given test, generated from:
+;
+; 1 extern int get_arg();
+; 2 extern void func(int x);
+; 3
+; 4 int main()
+; 5 {
+; 6   int a;
+; 7   func(get_arg());
+; 8 }
+; 9
+
+; The prologue-end line record is emitted with an incorrect associated address,
+; which causes a debugger to show the beginning of function body to be inside
+; the prologue.
+
+; This can be seen in the following trimmed assembler output:
+;
+; main:
+;   ...
+; # %bb.0:
+;   .loc  1 7 0 prologue_end
+;   pushq %rax
+;   .cfi_def_cfa_offset 16
+;   callq _Z7get_argv
+;   ...
+;   retq
+
+; The instruction 'pushq %rax' is part of the frame setup code.
+
+; The correct location for the prologue-end line information is just before
+; the call to '_Z7get_argv', as illustrated in the following trimmed
+; assembler output:
+;
+; main:
+;   ...
+; # %bb.0:
+;   pushq  %rax
+;   .cfi_def_cfa_offset 16
+;   .loc  1 7 0 prologue_end
+;   callq  _Z7get_argv
+;   ...
+;   retq
+
+; Check that the generated assembler matches the following sequence:
+
+; CHECK:      # %bb.0:
+; CHECK-NEXT:   pushq %rax
+; CHECK-NEXT:   .cfi_def_cfa_offset 16
+; CHECK-NEXT: .Ltmp0:
+; CHECK-NEXT:   .loc  1 7 8 prologue_end {{.*}}# invalid-prologue-end.cpp:7:8
+; CHECK-NEXT:   callq _Z7get_argv
+
+define i32 @main() #0 !dbg !7 {
+entry:
+  %a = alloca i32, align 4
+  call void @llvm.dbg.declare(metadata i32* %a, metadata !11, metadata !DIExpression()), !dbg !12
+  %call = call i32 @_Z7get_argv(), !dbg !13
+  call void @_Z4funci(i32 %call), !dbg !14
+  ret i32 0, !dbg !15
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+declare void @_Z4funci(i32) #2
+declare i32 @_Z7get_argv() #2
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 7.0.0 (trunk 322269)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "invalid-prologue-end.cpp", directory: "/home/carlos/llvm-root/work")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 7.0.0 (trunk 322269)"}
+!7 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 4, type: !8, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DILocalVariable(name: "a", scope: !7, file: !1, line: 6, type: !10)
+!12 = !DILocation(line: 6, column: 7, scope: !7)
+!13 = !DILocation(line: 7, column: 8, scope: !7)
+!14 = !DILocation(line: 7, column: 3, scope: !7)
+!15 = !DILocation(line: 8, column: 1, scope: !7)