[Bug default/25058] New: abigail does not recognize

[Bug default/25058] New: abigail does not recognize

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

            Bug ID: 25058
           Summary: abigail does not recognize
           Product: libabigail
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: simark at simark dot ca
                CC: libabigail at sourceware dot org
  Target Milestone: ---

When running abidiff on the provided libraries:

  abidiff liblttng-ctl2.10.so liblttng-ctl.so

some functions, like lttng_destruction_handle_wait_for_completion are
supposedly not referenced by the debug info, when in fact they are.  More
precisely, this is unexpected:

  3 Added function symbols not referenced by debug info:

     lttng_destruction_handle_wait_for_completion
     lttng_session_descriptor_destroy
     lttng_userspace_probe_location_destroy

It is correct that the function symbols are added in the new version of the
library, but it is incorrect that they are not referenced by debug info.

I traced down the problem to the fact that these functions' DIEs use
DW_AT_ranges to describe multiple PC ranges instead of DW_AT_low_pc to describe
a single range.  For example:

  0x0001bb61:   DW_TAG_subprogram
                  DW_AT_external  (true)
                  DW_AT_name     
("lttng_destruction_handle_wait_for_completion")
                  DW_AT_decl_file
("/tmp/tools-master/src/lttng-tools/src/lib/lttng-ctl/destruction-handle.c")
                  DW_AT_decl_line (237)
                  DW_AT_decl_column       (0x01)
                  DW_AT_prototyped        (true)
                  DW_AT_type      (0x0001a018
"lttng_destruction_handle_status")
                  DW_AT_ranges    (0x00002420
                     [0x0000000000018f20, 0x000000000001942d)
                     [0x0000000000011400, 0x000000000001140a))
                  DW_AT_frame_base        (DW_OP_call_frame_cfa)
                  DW_AT_GNU_all_call_sites        (true)
                  DW_AT_sibling   (0x0001c28e)

This makes abigail::dwarf_reader::read_context::get_function_address fail to
find the function address.  This makes build_function_decl fail to find the
corresponding ELF symbol, therefore not set the `is_in_public_symbol_table`
property of the function_decl.  This makes
corpus::exported_decls_builder::maybe_add_fn_to_exported_fns not add that
function to the functions described by debug info.

We would probably not see this if we built the binaries with -O0 (or at least
with -fno-reorder-blocks-and-partition), but it's still probably a bug that
abigail doesn't understand DW_AT_ranges.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12016|Library 1                   |liblttng-ctl2.10.so
        description|                            |

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #6 from dodji at redhat dot com ---
I confirm I can reproduce this issue.

And yes I agree that we should consider the first address expressed by the
value of the attribute DW_AT_ranges that is carried by the DW_TAG_subprogram
DIE that we are looking at, especially when there is no DW_AT_low_pc attribute
present.  That address would point to the ELF symbol of the function we are
looking at.

Thank you for taking the time to file this issue and sorry for the
inconvenience.

I am looking at this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[dodji at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

dodji at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from dodji at redhat dot com ---
This issue should now be fixed by commit
https://sourceware.org/git/gitweb.cgi?p=libabigail.git;a=commit;h=5b9f5efab2910641a9eefe3bcab39ad121bea5e8
that was pushed to the master branch of the Git repository.

It should be available in the 1.7 version of Libabigail.

Thank you for taking the time to file this issue and sorry for the
inconvenience.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #11 from Simon Marchi <simark at simark dot ca> ---
(In reply to dodji from comment #10)
> This issue should now be fixed by commit
> https://sourceware.org/git/gitweb.cgi?p=libabigail.git;a=commit;
> h=5b9f5efab2910641a9eefe3bcab39ad121bea5e8 that was pushed to the master
> branch of the Git repository.
> 
> It should be available in the 1.7 version of Libabigail.
> 
> Thank you for taking the time to file this issue and sorry for the
> inconvenience.

Thanks a lot for the quick fix!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #4 from Simon Marchi <simark at simark dot ca> ---
(In reply to Mark Wielaard from comment #3)
> Instead of checking for the Dw_AT_low_pc attribute value, libabigail should
> probably use libdw dwarf_ranges () which works for either low_pc/high_pc
> pairs (single range) or DW_AT_ranges/rnglists.

So, libabigail uses the function's low pc to try to find the ELF symbol
corresponding to the function DIE it is currently processing.  If it uses the
ranges, which address should it use to look up the ELF symbol?  Typically, one
range will correspond to the ".cold" version of the symbol and one range will
correspond to the symbol we are looking for.

I suppose it could try to look up an ELF symbol using the "low pc" of each
range, until it finds one whose name corresponds to the DW_AT_linkage_name of
the function DIE (if the DIE has that attribute).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[Dodji Seketeli]

"simark at simark dot ca" <sourceware-bugzilla@sourceware.org> a écrit:

> I suppose it could try to look up an ELF symbol using the "low pc" of each
> range, until it finds one whose name corresponds to the DW_AT_linkage_name of
> the function DIE (if the DIE has that attribute).

I think trying to match the DW_AT_linkage_name will lead to to errors
because we can't rely on the fact that a DW_AT_linkage_name has to match
a symbol name.

Symbol aliasing, which usually happens due to compiler code size
optimizations (e.g, happens a lot in C++), or due to assembler tricks
programmers play sometime, can make the name of the symbol be different
from the value of the DW_AT_linkage_name.

So instead, I'd go for just picking the first address (of the set
referred to by the value of the DW_AT_ranges attribute) that is for a
publicly exported function symbol and consider that one as being the one
we are looking for.

-- 
		Dodji

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #1 from Simon Marchi <simark at simark dot ca> ---
Created attachment 12016
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12016&action=edit
Library 1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12017|Library 2                   |liblttng-ctl.so
        description|                            |

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[dodji at seketeli dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #9 from dodji at seketeli dot org ---
"simark at simark dot ca" <sourceware-bugzilla@sourceware.org> a écrit:

> I suppose it could try to look up an ELF symbol using the "low pc" of each
> range, until it finds one whose name corresponds to the DW_AT_linkage_name of
> the function DIE (if the DIE has that attribute).

I think trying to match the DW_AT_linkage_name will lead to to errors
because we can't rely on the fact that a DW_AT_linkage_name has to match
a symbol name.

Symbol aliasing, which usually happens due to compiler code size
optimizations (e.g, happens a lot in C++), or due to assembler tricks
programmers play sometime, can make the name of the symbol be different
from the value of the DW_AT_linkage_name.

So instead, I'd go for just picking the first address (of the set
referred to by the value of the DW_AT_ranges attribute) that is for a
publicly exported function symbol and consider that one as being the one
we are looking for.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #7 from Simon Marchi <simark at simark dot ca> ---
(In reply to dodji from comment #6)
> I confirm I can reproduce this issue.
> 
> And yes I agree that we should consider the first address expressed by the
> value of the attribute DW_AT_ranges that is carried by the DW_TAG_subprogram
> DIE that we are looking at, especially when there is no DW_AT_low_pc
> attribute present.  That address would point to the ELF symbol of the
> function we are looking at.

Just to clarify: by looking up the ELF symbol associated to the low pc of the
_first_ range may not be sufficient, as it could give you the wrong ELF symbol.

In liblttng-ctl.so, the function lttng_destruction_handle_wait_for_completion
has these ranges:

      DW_AT_ranges    (0x00002420
         [0x0000000000018f20, 0x000000000001942d)
         [0x0000000000011400, 0x000000000001140a))

And that corresponds to these two symbols:

   585: 0000000000018f20  1293 FUNC    GLOBAL DEFAULT   12
lttng_destruction_handle_wait_for_completion
    39: 0000000000011400    10 FUNC    LOCAL  DEFAULT   12
lttng_destruction_handle_wait_for_completion.cold

In that case, looking up the low pc of the first range, 0x18f20, would find the
right symbol.  But it would also be valid for the ranges to be in the other
order, like this:

      DW_AT_ranges    (0x00002420
         [0x0000000000011400, 0x000000000001140a)
         [0x0000000000018f20, 0x000000000001942d))

In that case, you would find lttng_destruction_handle_wait_for_completion.cold,
which is not an exported symbol, and maybe wrongfully conclude that this is not
an exposed function.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[Dodji Seketeli]

"simark at simark dot ca" <sourceware-bugzilla@sourceware.org> a écrit:

> Just to clarify: by looking up the ELF symbol associated to the low pc of the
> _first_ range may not be sufficient, as it could give you the wrong ELF symbol.

[...]

Right, I think we should get the first address that corresponds to a
function symbol that is defined and publicly exported.

I initially wrote a patch that was just taking the first address of the
range referred to by DW_AT_ranges, and then I read your comment
addressing this issue and I updated the patch to make sure the address
we are getting (from the range) is for a publicly exported function
symbol.

Thanks for the discussion.

-- 
		Dodji

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #5 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Simon Marchi from comment #4)
> I suppose it could try to look up an ELF symbol using the "low pc" of each
> range, until it finds one whose name corresponds to the DW_AT_linkage_name
> of the function DIE (if the DIE has that attribute).

I don't know the libabigail code well enough to be able to say that it needs to
use the linkage_name attribute for that. But yes, instead of using the low_pc
attribute directly, I would expect it would use the start of the range
address(es) to match against the ELF symbol addresses.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|abigail does not recognize  |abigail does not handle
                   |                            |subprogram DIEs with
                   |                            |DW_AT_ranges instead of
                   |                            |DW_AT_low_pc

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[dodji at seketeli dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #8 from dodji at seketeli dot org ---
"simark at simark dot ca" <sourceware-bugzilla@sourceware.org> a écrit:

> Just to clarify: by looking up the ELF symbol associated to the low pc of the
> _first_ range may not be sufficient, as it could give you the wrong ELF symbol.

[...]

Right, I think we should get the first address that corresponds to a
function symbol that is defined and publicly exported.

I initially wrote a patch that was just taking the first address of the
range referred to by DW_AT_ranges, and then I read your comment
addressing this issue and I updated the patch to make sure the address
we are getting (from the range) is for a publicly exported function
symbol.

Thanks for the discussion.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

--- Comment #2 from Simon Marchi <simark at simark dot ca> ---
Created attachment 12017
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12017&action=edit
Library 2

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25058] abigail does not handle subprogram DIEs with DW_AT_ranges instead of DW_AT_low_pc

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25058

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
Instead of checking for the Dw_AT_low_pc attribute value, libabigail should
probably use libdw dwarf_ranges () which works for either low_pc/high_pc pairs
(single range) or DW_AT_ranges/rnglists.

-- 
You are receiving this mail because:
You are on the CC list for the bug.