Bug 105024 - VLAs with C++ causes the gimplifier to produce __builtin_stack_save with the line# of the closing }
Summary: VLAs with C++ causes the gimplifier to produce __builtin_stack_save with the ...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 11.2.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-debug
Depends on:
Blocks:
 
Reported: 2022-03-22 18:50 UTC by blarsen
Modified: 2022-03-23 10:14 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-03-22 00:00:00


Attachments
minimal testcase (291 bytes, text/plain)
2022-03-22 18:50 UTC, blarsen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description blarsen 2022-03-22 18:50:43 UTC
Created attachment 52665 [details]
minimal testcase

When compiling the attached test case with the options -g -O0, there is a bug in the line table generation.

The first instruction of merge_sort is assigned to line 17 instead of line 2, then backtracks to line 5. It generates some confusion when trying to debug the code, as GDB prints the wrong line when that instruction is being evaluated.

My guess is that g++ sees the early exit and assumes that part of the "if" is related to that exit, and mislabels that instruction. I tried removing the "return" statement and putting the rest of the code inside an else block, and the line table was closer to what I expected.

This seems to be a long standing bug, as I can reproduce it with g++ 8.5.0 and g++ 11.2.1, though 8.5.0 sets the line as 16 instead.
Comment 1 Andrew Pinski 2022-03-22 23:35:47 UTC
  [/app/example.cpp:17:1] saved_stack.15 = __builtin_stack_save ();
Comment 2 Andrew Pinski 2022-03-22 23:37:30 UTC
Note the C front-end produces:
  [/app/example.cpp:1:34] saved_stack.10 = __builtin_stack_save ();

This is all VLA related which means this is using an extension to C++.
Comment 3 Jakub Jelinek 2022-03-23 10:03:03 UTC
For the __builtin_stack_save, the locus comes from the bind_expr or its block:
gimplify_bind_expr has:
  /* Source location wise, the cleanup code (stack_restore and clobbers)
     belongs to the end of the block, so propagate what we have.  The
     stack_save operation belongs to the beginning of block, which we can
     infer from the bind_expr directly if the block has no explicit
     assignment.  */
  if (BIND_EXPR_BLOCK (bind_expr))
    {
      end_locus = BLOCK_SOURCE_END_LOCATION (BIND_EXPR_BLOCK (bind_expr));
      start_locus = BLOCK_SOURCE_LOCATION (BIND_EXPR_BLOCK (bind_expr));
    }
  if (start_locus == 0)
    start_locus = EXPR_LOCATION (bind_expr);
Apparently, neither in C nor in C++ we have BLOCK_SOURCE_*LOCATION on the BIND_EXPR_BLOCK of this bind_expr, so it comes from EXPR_LOCATION of the bind_expr.
The C FE sets it from:
#0  0x0000000000bc42bb in c_build_bind_expr (loc=243168, block=<block 0x7fffea380480>, body=<statement_list 0x7fffea38c200>) at ../../gcc/c-family/c-gimplify.cc:661
#1  0x0000000000aed271 in c_end_compound_stmt (loc=243168, stmt=<statement_list 0x7fffea38c200>, do_scope=true) at ../../gcc/c/c-typeck.cc:11537
#2  0x0000000000b14a8c in c_parser_compound_statement (parser=0x7fffea24db40, endlocp=0x7fffffffd38c) at ../../gcc/c/c-parser.cc:5613
where the used location is the location of the opening {
5599	  brace_loc = c_parser_peek_token (parser)->location;
5600	  if (!c_parser_require (parser, CPP_OPEN_BRACE, "expected %<{%>"))
While C++ FE sets it in:
#0  0x0000000000f882a1 in c_build_bind_expr (loc=307648, block=<block 0x7fffea3ae5a0>, body=<statement_list 0x7fffea39f8e0>) at ../../gcc/c-family/c-gimplify.cc:661
#1  0x0000000000e7bcfd in do_poplevel (stmt_list=<statement_list 0x7fffea39f8e0>) at ../../gcc/cp/semantics.cc:639
#2  0x0000000000e7fdb0 in finish_compound_stmt (stmt=<statement_list 0x7fffea39f8e0>) at ../../gcc/cp/semantics.cc:1782
where do_poplevel uses input_location which is the "current" location, so the end of the block and the location of the { is not available at that point.
Comment 4 Jakub Jelinek 2022-03-23 10:14:19 UTC
Note, cp_parser_compound_statement knows the opening { location, e.g. in
braces.m_open_loc, so we'd need to change finish_compound_stmt and do_poplevel to take a location_t argument that would carry the opening { location if any (and fallback there to input_location which will keep current behavior if we can't figure something better).
Anyway, this isn't a regression, so GCC 13 material.