Bug 54218 - Debug info for function parameters is incorrect when compiled with -O0
Summary: Debug info for function parameters is incorrect when compiled with -O0
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-debug
Depends on:
Blocks:
 
Reported: 2012-08-10 08:37 UTC by Senthil Kumar Selvaraj
Modified: 2013-01-07 12:06 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-08-10 00:00:00


Attachments
Failing dejagnu test case (169 bytes, text/x-c)
2012-08-10 08:37 UTC, Senthil Kumar Selvaraj
Details
Failing dejagnu test case (right line number) (180 bytes, text/x-c)
2012-08-10 10:25 UTC, Senthil Kumar Selvaraj
Details
Draft Patch for the fix of 54218 (367 bytes, patch)
2013-01-07 12:04 UTC, George Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Senthil Kumar Selvaraj 2012-08-10 08:37:32 UTC
Created attachment 27980 [details]
Failing dejagnu test case

When compiling a function, like below, with no optimizations (-O0), on a x86_64 target (also occurs for atleast one other target (avr))

void func(int p)
{
    p = 0; 
    p = 32;
}

wrong debug information is emitted for the function parameter (p), that cause the debugger to keep showing the value of the actual argument, even after p is overwritten. A failing dejagnu test is attached.

Based on a preliminary analysis, the actual problem seems to be that stack space for a function parameter (that is not used??) is allocated twice when gimple_expand_cfg runs - once when expand_used_vars runs, and later when assign_parm_setup_stack runs. expand_used_vars allocates stack space despite the check for a default definition being present in the partition, because the function parameter node (which I guess should be the default definition) is not present when iterating over num_ssa_names.
Comment 1 Richard Biener 2012-08-10 08:53:46 UTC
That's because the actual parameter value is not used:

func (int p)
{
;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  p_1 = 0;
  p_2 = 32;
  return;

Partition 0 (p_1 - 1 2 )


Does -fvar-tracking "fix" it?
Comment 2 Senthil Kumar Selvaraj 2012-08-10 10:23:55 UTC
Comment on attachment 27980 [details]
Failing dejagnu test case

>/* { dg-do run } */
>/* { dg-options "-g" } */
>/* { dg-skip-if "" { *-*-* }  { "*" } { "-O0" } } */
>
>void func(int p)
>{
>    p = 0; /* { dg-final { gdb-test 8 "p" "0" } } */
>    p = 32;/* { dg-final { gdb-test 8 "p" "42" } } */
>}
>
>int
>main (void)
>{
>    int local = 42;
>    func(local);
>}
Comment 3 Senthil Kumar Selvaraj 2012-08-10 10:25:03 UTC
Created attachment 27981 [details]
Failing dejagnu test case (right line number)
Comment 4 Senthil Kumar Selvaraj 2012-08-10 10:39:32 UTC
(In reply to comment #1)
> That's because the actual parameter value is not used:
> 
> func (int p)
> {
> ;;   basic block 2, loop depth 0
> ;;    pred:       ENTRY
>   p_1 = 0;
>   p_2 = 32;
>   return;
> 
> Partition 0 (p_1 - 1 2 )
> 
> 
> Does -fvar-tracking "fix" it?

Yes, it does. Doesn't change the code generated though - the initial copy is still at a different frame offset.

(In reply to comment #1)
> That's because the actual parameter value is not used:
> 
> func (int p)
> {
> ;;   basic block 2, loop depth 0
> ;;    pred:       ENTRY
>   p_1 = 0;
>   p_2 = 32;
>   return;
> 
> Partition 0 (p_1 - 1 2 )
> 
> 
> Does -fvar-tracking "fix" it?

(In reply to comment #1)
> That's because the actual parameter value is not used:
> 
> func (int p)
> {
> ;;   basic block 2, loop depth 0
> ;;    pred:       ENTRY
>   p_1 = 0;
>   p_2 = 32;
>   return;
> 
> Partition 0 (p_1 - 1 2 )
> 
> 
> Does -fvar-tracking "fix" it?
Comment 5 George Thomas 2013-01-07 12:04:22 UTC
Created attachment 29095 [details]
Draft Patch for the fix of 54218

The issue is happening because stack space is allocated twice

1. assign_params_setup_stack and
2. expand_used_vars

The proposed fix is to allocate the space only once in assign_params_stack
by explicitly checking in expand_used_vars if the tree node is of type PARM_DECL.

if its PARM_DECL, it would mean that it would already have been expanded and
hence do not require further expansion.

This fixes the issue and allows debugging to work properly.

I would like to know if it would be an acceptable change.
Comment 6 George Thomas 2013-01-07 12:06:01 UTC
Another alternative thought of for the fix was to,

Make the Parameters to be the default def of the next use. In this case,
the condition mentioned above will not expand the variable.

if (!bitmap_bit_p (SA.partition_has_default_def, i))
  {
    expand_one_var (var, true, true);
    gcc_assert (SA.partition_to_pseudo[i]);
  }