| Message ID | 15074732.mILDtaeVB4@polaris |
|---|---|
| State | New |
| Headers | show |
On Sun, Jun 16, 2013 at 12:51:25PM +0200, Eric Botcazou wrote: > the subject is slightly misleading since it's actually about structures with > integral modes which are passed by reference on some platforms, e.g. SPARC or > PowerPC 32-bit. There are 3 issues: > > 1. At -O0, the parameters of this kind are not homed on the stack, which > means that backtraces can display <optimized out> for them. > > 2. Since the fix for PR debug/48163, the locations emitted at -O1 and above > for them are totally bogus (there is a bogus additional dereference). > > 3. The location lists are quickly wrong for them because they fail to take > into account that the register parameter is clobbered. > > The attached patch addresses 1. and 2. fully and 3. partially (for now). It > has been tested on x86-64 and PowerPC/Linux. OK for the mainline? This really should come with testcases (e.g. guality ones). Jakub
> This really should come with testcases (e.g. guality ones).
guality testcases are barely maintainable, especially on non-x86 platforms, so
I'd rather not enter this business. I'll discuss with Joel and see whether we
can coordinate with the GDB side.
On Mon, Jun 17, 2013 at 09:46:30AM +0200, Eric Botcazou wrote: > > This really should come with testcases (e.g. guality ones). > > guality testcases are barely maintainable, especially on non-x86 platforms, so > I'd rather not enter this business. I'll discuss with Joel and see whether we > can coordinate with the GDB side. Especially if it is -O0 only, I don't see why you think so. Just dg-skip-if it for -O1+ if you believe it is unreliable for some reason, but if you look at the parameter value after the prologue, not showing the right value at -O0 would be a serious bug everywhere. Having some GDB testcase also doesn't hurt, but having it in GCC testsuite has significant advantages over that, most GCC developers don't run GDB testsuite after any changes they do. Jakub
> Especially if it is -O0 only, I don't see why you think so. Just dg-skip-if > it for -O1+ if you believe it is unreliable for some reason, but if you > look at the parameter value after the prologue, not showing the right value > at -O0 would be a serious bug everywhere. Having some GDB testcase also > doesn't hurt, but having it in GCC testsuite has significant advantages > over that, most GCC developers don't run GDB testsuite after any changes > they do. All right, I've attached a couple of guality testcases (param-1.c for -O0 and param-2.c for -O1 -fno-var-tracking-assignments, a param-3.c for bare -O1 will require further adjustments in var-tracking.c). Unfortunately they don't fail without the patch e.g. on PowerPC, they are reported as unsupported instead because of "Cannot access memory at address" messages from GDB... * gcc.dg/guality/param-1.c: New test. * gcc.dg/guality/param-2.c: Likewise.
Index: function.c =================================================================== --- function.c (revision 200122) +++ function.c (working copy) @@ -3084,17 +3084,27 @@ assign_parm_setup_reg (struct assign_par emit_move_insn (parmreg, validated_mem); /* If we were passed a pointer but the actual value can safely live - in a register, put it in one. */ - if (data->passed_pointer - && TYPE_MODE (TREE_TYPE (parm)) != BLKmode - /* If by-reference argument was promoted, demote it. */ - && (TYPE_MODE (TREE_TYPE (parm)) != GET_MODE (DECL_RTL (parm)) - || use_register_for_decl (parm))) + in a register, retrieve it and use it directly. */ + if (data->passed_pointer && TYPE_MODE (TREE_TYPE (parm)) != BLKmode) { /* We can't use nominal_mode, because it will have been set to Pmode above. We must use the actual mode of the parm. */ - parmreg = gen_reg_rtx (TYPE_MODE (TREE_TYPE (parm))); - mark_user_reg (parmreg); + if (use_register_for_decl (parm)) + { + parmreg = gen_reg_rtx (TYPE_MODE (TREE_TYPE (parm))); + mark_user_reg (parmreg); + } + else + { + int align = STACK_SLOT_ALIGNMENT (TREE_TYPE (parm), + TYPE_MODE (TREE_TYPE (parm)), + TYPE_ALIGN (TREE_TYPE (parm))); + parmreg + = assign_stack_local (TYPE_MODE (TREE_TYPE (parm)), + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (parm))), + align); + set_mem_attributes (parmreg, parm, 1); + } if (GET_MODE (parmreg) != GET_MODE (DECL_RTL (parm))) { Index: var-tracking.c =================================================================== --- var-tracking.c (revision 200122) +++ var-tracking.c (working copy) @@ -5836,7 +5836,24 @@ add_stores (rtx loc, const_rtx expr, voi { rtx xexpr = gen_rtx_SET (VOIDmode, loc, src); if (same_variable_part_p (src, REG_EXPR (loc), REG_OFFSET (loc))) - mo.type = MO_COPY; + { + /* If this is an instruction copying (part of) a parameter + passed by invisible reference to its register location, + pretend it's a SET so that the initial memory location + is discarded, as the parameter register can be reused + for other purposes and we do not track locations based + on generic registers. */ + if (MEM_P (src) + && REG_EXPR (loc) + && TREE_CODE (REG_EXPR (loc)) == PARM_DECL + && DECL_MODE (REG_EXPR (loc)) != BLKmode + && MEM_P (DECL_INCOMING_RTL (REG_EXPR (loc))) + && XEXP (DECL_INCOMING_RTL (REG_EXPR (loc)), 0) + != arg_pointer_rtx) + mo.type = MO_SET; + else + mo.type = MO_COPY; + } else mo.type = MO_SET; mo.u.loc = xexpr; @@ -9086,7 +9103,7 @@ emit_notes_in_bb (basic_block bb, datafl else var_mem_set (set, loc, VAR_INIT_STATUS_UNINITIALIZED, NULL); - emit_notes_for_changes (insn, EMIT_NOTE_AFTER_INSN, set->vars); + emit_notes_for_changes (insn, EMIT_NOTE_BEFORE_INSN, set->vars); } break; @@ -9533,12 +9550,11 @@ vt_add_function_parameter (tree parm) if (!vt_get_decl_and_offset (incoming, &decl, &offset)) { - if (REG_P (incoming) || MEM_P (incoming)) + if (MEM_P (incoming)) { /* This means argument is passed by invisible reference. */ offset = 0; decl = parm; - incoming = gen_rtx_MEM (GET_MODE (decl_rtl), incoming); } else {