Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Simpler DWARF for C99 VLAs #35295

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR36322
Status NEW
Importance P enhancement
Reported by Paul Robinson (paul_robinson@playstation.sony.com)
Reported on 2018-02-09 10:38:14 -0800
Last modified on 2018-10-25 20:11:56 -0700
Version unspecified
Hardware PC Windows NT
CC aprantl@apple.com, dblaikie@gmail.com, hfinkel@anl.gov, international.phantom@gmail.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk, sander.desmalen@arm.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Filed on behalf of Carlos Enciso, who made this post-commit suggestion
on Phabricator D41698:

Hi @sdesmalen!

For the following test case

  int main() {
    int size = 2;

    int var[size];
    var[1] = 1;

    return 0;
  }

I compared the DWARF generated by GCC and it looks like

  DW_TAG_variable "var"
    DW_AT_location ...
    DW_AT_type DW_FORM_ref4
      DW_TAG_array_type
        DW_AT_type -> "int"
        DW_TAG_subrange_type
          DW_AT_type -> "sizetype"
          DW_AT_upper_bound DW_FORM_exprloc [4] = { DW_OP_fbreg 0xffffffb8 DW_OP_deref }

GCC use DW_AT_upper_bound with an associated location expression to describe
the VLA boundaries.

In order to reduce the side effects created by the artifical-variable as
described in my previous comment
(https://bugs.llvm.org/show_bug.cgi?id=30553#c3) and to keep the generated
DWARF within a reasonable size, I would suggest the GCC aproach as a size
optimization.

The DWARF description of the artificial-variable could be removed and its
location expression used by the array's subrange_type, instead of the
subrange_type making a reference to the artificial-variable.
Quuxplusone commented 6 years ago
Currently (after r323952) we emit an artificial variable that holds the
count of array elements.  As a variant of Carlos' suggestion, we could
emit the appropriate DWARF expression as the count, instead of conjuring
up an actual DW_TAG_variable.

This is a small size optimization, whose effect depends on the number of
VLAs used in a particular bit of source.

I'm mildly curious what happens if a function with a VLA gets inlined
into another function; the bounds expression is part of the type, but
the location would presumably be different for each inlined instance.
Quuxplusone commented 6 years ago
> I'm mildly curious what happens if a function with a VLA gets inlined
> into another function; the bounds expression is part of the type, but
> the location would presumably be different for each inlined instance.

If anyone wants to play with this, we have a testcase for that very situation in

  llvm/test/DebugInfo/X86/vla-dependencies.ll

And I would be really surprised if any debugger interpreted this correctly.
Quuxplusone commented 6 years ago
(In reply to Adrian Prantl from comment #2)
> > I'm mildly curious what happens if a function with a VLA gets inlined
> > into another function; the bounds expression is part of the type, but
> > the location would presumably be different for each inlined instance.
>
> If anyone wants to play with this, we have a testcase for that very
> situation in
>
>   llvm/test/DebugInfo/X86/vla-dependencies.ll
>
> And I would be really surprised if any debugger interpreted this correctly.

Let me spin off "other issues with the VLA implementation" as its
own bug, we can keep this one specifically for the suggestion to
avoid emitting a separate DIE entirely.