[1/5] Don't enable -gvariable-location-views by default for DWARF5.

Message ID 20200824125658.22526-2-mark@klomp.org
State New
Headers show
Series
  • [1/5] Don't enable -gvariable-location-views by default for DWARF5.
Related show

Commit Message

Mark Wielaard Aug. 24, 2020, 12:56 p.m.
DWARF5 makes it possible to read loclists tables without consulting
the debuginfo tree by introducing a table header. Adding location views
breaks this (at least for binutils and elfutils). So don't enable
variable-location-views by default if DWARF5 or higher is selected.
---
 gcc/doc/invoke.texi | 6 +++---
 gcc/toplev.c        | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.18.4

Comments

Jakub Jelinek via Gcc-patches Aug. 24, 2020, 5:38 p.m. | #1
On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:
> DWARF5 makes it possible to read loclists tables without consulting

> the debuginfo tree by introducing a table header. Adding location views

> breaks this (at least for binutils and elfutils). So don't enable

> variable-location-views by default if DWARF5 or higher is selected.


This should be discussed with Alex, CCed.
I'd say elfutils/binutils should just show .debug_loclists independent of
.debug_info if there are no loc views and use .debug_info otherwise.

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi

> index 70dc1ab73a12..e5dddc236d7d 100644

> --- a/gcc/doc/invoke.texi

> +++ b/gcc/doc/invoke.texi

> @@ -9301,9 +9301,9 @@ can be consumed by debug information consumers that are not aware of

>  these augmentations, but they won't derive any benefit from them either.

>  

>  This is enabled by default when outputting DWARF 2 debug information at

> -the normal level, as long as there is assembler support,

> -@option{-fvar-tracking-assignments} is enabled and

> -@option{-gstrict-dwarf} is not.  When assembler support is not

> +the normal level for DWARF versions lower than 5, as long as there

> +is assembler support, @option{-fvar-tracking-assignments} is enabled

> +and @option{-gstrict-dwarf} is not.  When assembler support is not

>  available, this may still be enabled, but it will force GCC to output

>  internal line number tables, and if

>  @option{-ginternal-reset-location-views} is not enabled, that will most

> diff --git a/gcc/toplev.c b/gcc/toplev.c

> index 07457d08c3aa..34218c6b3349 100644

> --- a/gcc/toplev.c

> +++ b/gcc/toplev.c

> @@ -1672,6 +1672,7 @@ process_options (void)

>  	   && (write_symbols == DWARF2_DEBUG

>  	       || write_symbols == VMS_AND_DWARF2_DEBUG)

>  	   && !dwarf_strict

> +	   && dwarf_version < 5

>  	   && dwarf2out_as_loc_support

>  	   && dwarf2out_as_locview_support);

>      }

> -- 

> 2.18.4


	Jakub
Mark Wielaard Aug. 24, 2020, 8:12 p.m. | #2
Hi,

On Mon, Aug 24, 2020 at 07:38:10PM +0200, Jakub Jelinek wrote:
> On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:

> > DWARF5 makes it possible to read loclists tables without consulting

> > the debuginfo tree by introducing a table header. Adding location views

> > breaks this (at least for binutils and elfutils). So don't enable

> > variable-location-views by default if DWARF5 or higher is selected.

> 

> This should be discussed with Alex, CCed.

> I'd say elfutils/binutils should just show .debug_loclists independent of

> .debug_info if there are no loc views and use .debug_info otherwise.


Yes, sorry, should have CCed Alex. I do agree this is less than
ideal. It was really just to make sure the elfutils testsuite was
clean.

But the issue is real. As is, when enabling DWARF5, it is impossible
for consumers to parse the .debug_loclists independently. The only way
to know whether (and where) the locviews are is by finding and then
parsing the corresponding CU DIE tree.

There is actually an alternative representation enabled by
-gvariable-location-views=incompat5 that might help, but as the name
implies it isn't standard DWARF5, and I believe neither elfutils nor
binutils parses it.

I am not sure what a good default is in case we enable -gdwarf-5.

Cheers,

Mark
Alexandre Oliva Aug. 25, 2020, 4:05 a.m. | #3
On Aug 24, 2020, Jakub Jelinek <jakub@redhat.com> wrote:

> On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:

>> DWARF5 makes it possible to read loclists tables without consulting

>> the debuginfo tree by introducing a table header. Adding location views

>> breaks this (at least for binutils and elfutils). So don't enable

>> variable-location-views by default if DWARF5 or higher is selected.


> This should be discussed with Alex, CCed.

> I'd say elfutils/binutils should just show .debug_loclists independent of

> .debug_info if there are no loc views and use .debug_info otherwise.


I've suggested before that it made sense to me to start emitting
locviews when there were concrete plans to implement support for them in
debug info consumers.

Without such plans, it would make more sense to just disable them
altogether.

Now, if there are any such plans, it is disabling them for the default
debug format that doesn't make much sense to me; it would seem to make
more sense to adopt and promote the proposed extension, implemented in
=incompat5 in GCC, but it would need some implementation work in
consumers to at least ignore the extension.


Red Hat has had me involved in these efforts for over a decade, but I'm
not aware of its plans any more, and I don't know of anyone else driving
the implementation of locviews in consumers, so, given the little I
know, this patch seems like a timid step in a reasonable direction:
outputting locviews is no use if there are no consumers for it, more so
when they actively disturb existing standard-compliant consumers.

-- 
Alexandre Oliva, happy hacker
https://FSFLA.org/blogs/lxo/
Free Software Activist
GNU Toolchain Engineer
Jakub Jelinek via Gcc-patches Aug. 25, 2020, 7:27 a.m. | #4
On Tue, Aug 25, 2020 at 6:05 AM Alexandre Oliva <oliva@adacore.com> wrote:
>

> On Aug 24, 2020, Jakub Jelinek <jakub@redhat.com> wrote:

>

> > On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:

> >> DWARF5 makes it possible to read loclists tables without consulting

> >> the debuginfo tree by introducing a table header. Adding location views

> >> breaks this (at least for binutils and elfutils). So don't enable

> >> variable-location-views by default if DWARF5 or higher is selected.

>

> > This should be discussed with Alex, CCed.

> > I'd say elfutils/binutils should just show .debug_loclists independent of

> > .debug_info if there are no loc views and use .debug_info otherwise.

>

> I've suggested before that it made sense to me to start emitting

> locviews when there were concrete plans to implement support for them in

> debug info consumers.

>

> Without such plans, it would make more sense to just disable them

> altogether.

>

> Now, if there are any such plans, it is disabling them for the default

> debug format that doesn't make much sense to me; it would seem to make

> more sense to adopt and promote the proposed extension, implemented in

> =incompat5 in GCC, but it would need some implementation work in

> consumers to at least ignore the extension.

>

>

> Red Hat has had me involved in these efforts for over a decade, but I'm

> not aware of its plans any more, and I don't know of anyone else driving

> the implementation of locviews in consumers, so, given the little I

> know, this patch seems like a timid step in a reasonable direction:

> outputting locviews is no use if there are no consumers for it, more so

> when they actively disturb existing standard-compliant consumers.


But then leaving it enabled by default but not with -gdwarf-5 looks odd
and backward to me as well.  If the consensus is there's no use then
please disable them by default everywhere instead.

Richard.

> --

> Alexandre Oliva, happy hacker

> https://FSFLA.org/blogs/lxo/

> Free Software Activist

> GNU Toolchain Engineer
Mark Wielaard Aug. 25, 2020, 9:24 a.m. | #5
Hi Alex,

On Tue, 2020-08-25 at 01:05 -0300, Alexandre Oliva wrote:
> it would seem to

> make more sense to adopt and promote the proposed extension,

> implemented in =incompat5 in GCC, but it would need some

> implementation work in consumers to at least ignore the extension.


Is that what is described in:
http://www.fsfla.org/~lxoliva/papers/sfn/dwarf6-sfn-lvu.txt

Does it match the proposal in:
http://www.dwarfstd.org/ShowIssue.php?issue=170427.1

Should we try to introduce your extending loclists proposal:
http://www.dwarfstd.org/ShowIssue.php?issue=170427.2

The main issue is that there is no formal way of extending the
loclists.

Thanks,

Mark
Alexandre Oliva Aug. 26, 2020, 12:22 p.m. | #6
Hello, Mark,

On Aug 25, 2020, Mark Wielaard <mark@klomp.org> wrote:

> On Tue, 2020-08-25 at 01:05 -0300, Alexandre Oliva wrote:

>> it would seem to

>> make more sense to adopt and promote the proposed extension,

>> implemented in =incompat5 in GCC, but it would need some

>> implementation work in consumers to at least ignore the extension.


> Is that what is described in:

> http://www.fsfla.org/~lxoliva/papers/sfn/dwarf6-sfn-lvu.txt


> Does it match the proposal in:

> http://www.dwarfstd.org/ShowIssue.php?issue=170427.1


Yes, the implementation of incompat5 is supposed to match the
enhancement to DWARF proposed in this issue.

> Should we try to introduce your extending loclists proposal:

> http://www.dwarfstd.org/ShowIssue.php?issue=170427.2


If it makes sense to you, sure.  I think it could be useful for loclist
compression, besides adding extensibility to an unusually rigid part of
DWARF specs.  I'm not attached to this particular formulation, though,
and I'm not aware of any implementations thereof.  I only felt the need
for extending loclists and found myself severely constrained.

> The main issue is that there is no formal way of extending the

> loclists.


*nod*

Thanks,

-- 
Alexandre Oliva, happy hacker
https://FSFLA.org/blogs/lxo/
Free Software Activist
GNU Toolchain Engineer
Mark Wielaard Sept. 29, 2020, 11:15 a.m. | #7
Hi,

On Mon, 2020-08-24 at 19:38 +0200, Jakub Jelinek wrote:
> On Mon, Aug 24, 2020 at 02:56:54PM +0200, Mark Wielaard wrote:

> > DWARF5 makes it possible to read loclists tables without consulting

> > the debuginfo tree by introducing a table header. Adding location

> > views

> > breaks this (at least for binutils and elfutils). So don't enable

> > variable-location-views by default if DWARF5 or higher is selected.

> 

> This should be discussed with Alex, CCed.

> I'd say elfutils/binutils should just show .debug_loclists

> independent of

> .debug_info if there are no loc views and use .debug_info otherwise.


So it turned out that it was really bugs in elfutils and binutils.
For elfutils it now tracks locviews in .debug_loclists just like it did
for .debug_loc:
https://sourceware.org/pipermail/elfutils-devel/2020q3/002900.html

For binutils it actually tracked locviews correctly, but didn't handle
DW_LLE_start_end and DW_LLE_start_length. Patch submitted:
https://sourceware.org/pipermail/binutils/2020-September/113510.html

For tools that access the location lists (and locviews) through the DIE
attributes this never was an issue.

Patch retracted.

Cheers,

Mark

Patch

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 70dc1ab73a12..e5dddc236d7d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9301,9 +9301,9 @@  can be consumed by debug information consumers that are not aware of
 these augmentations, but they won't derive any benefit from them either.
 
 This is enabled by default when outputting DWARF 2 debug information at
-the normal level, as long as there is assembler support,
-@option{-fvar-tracking-assignments} is enabled and
-@option{-gstrict-dwarf} is not.  When assembler support is not
+the normal level for DWARF versions lower than 5, as long as there
+is assembler support, @option{-fvar-tracking-assignments} is enabled
+and @option{-gstrict-dwarf} is not.  When assembler support is not
 available, this may still be enabled, but it will force GCC to output
 internal line number tables, and if
 @option{-ginternal-reset-location-views} is not enabled, that will most
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 07457d08c3aa..34218c6b3349 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1672,6 +1672,7 @@  process_options (void)
 	   && (write_symbols == DWARF2_DEBUG
 	       || write_symbols == VMS_AND_DWARF2_DEBUG)
 	   && !dwarf_strict
+	   && dwarf_version < 5
 	   && dwarf2out_as_loc_support
 	   && dwarf2out_as_locview_support);
     }