[Lldb-commits] [PATCH] Fixed symbol ordering to be stable with overlapping symbols.

[Greg Clayton]

Looks good. I would also recommend a patch that makes sure that the most important symbols for a given address are listed first in symbol tables. This is a job for each ObjectFile subclass (ObjectFileMachO, ObjectFileELF, ObjectFileCOFF, etc). On MacOSX for example, we have alias symbols that are marked with extra flags. So if "printf" is a symbol at 0x1000, but it has a "printf$UNIX2003" variant, we should make sure the alias symbol isn't the first one that is shown. Not sure if there is anything similar in ELF or COFF, but it may be worth checking in to.

Greg

On Sep 16, 2013, at 1:07 PM, Richard Mitton <richard at codersnotes.com> wrote:

> If two symbols A and B have the same address, then doing lookups at that address will return different symbols depending on whether an offset was used.
> 
> e.g.  if A==B==0x100000
> 0x100000 -> A
> 0x100007 -> B
> 
> This would result in spurious labels when disassembling a function with an alias. This patch fixes it by ensuring we always return the first symbol covering an address.
> 
> 
> http://llvm-reviews.chandlerc.com/D1690
> 
> Files:
>  include/lldb/Core/RangeMap.h
> 
> Index: include/lldb/Core/RangeMap.h
> ===================================================================
> --- include/lldb/Core/RangeMap.h
> +++ include/lldb/Core/RangeMap.h
> @@ -1229,16 +1229,11 @@
>                 typename Collection::const_iterator end = m_entries.end();
>                 typename Collection::const_iterator pos = std::lower_bound (begin, end, entry, BaseLessThan);
> 
> +                while(pos != begin && pos[-1].Contains(addr))
> +                    --pos;
> +
>                 if (pos != end && pos->Contains(addr))
> -                {
>                     return std::distance (begin, pos);
> -                }
> -                else if (pos != begin)
> -                {
> -                    --pos;
> -                    if (pos->Contains(addr))
> -                        return std::distance (begin, pos);
> -                }
>             }
>             return UINT32_MAX;
>         }
> @@ -1257,19 +1252,12 @@
>                 typename Collection::iterator begin = m_entries.begin();
>                 typename Collection::iterator end = m_entries.end();
>                 typename Collection::iterator pos = std::lower_bound (begin, end, entry, BaseLessThan);
> +
> +                while(pos != begin && pos[-1].Contains(addr))
> +                    --pos;
> 
>                 if (pos != end && pos->Contains(addr))
> -                {
>                     return &(*pos);
> -                }
> -                else if (pos != begin)
> -                {
> -                    --pos;
> -                    if (pos->Contains(addr))
> -                    {
> -                        return &(*pos);
> -                    }
> -                }
>             }
>             return NULL;
>         }
> @@ -1288,18 +1276,11 @@
>                 typename Collection::const_iterator end = m_entries.end();
>                 typename Collection::const_iterator pos = std::lower_bound (begin, end, entry, BaseLessThan);
> 
> +                while(pos != begin && pos[-1].Contains(addr))
> +                    --pos;
> +
>                 if (pos != end && pos->Contains(addr))
> -                {
> -                    return &(*pos); 
> -                }
> -                else if (pos != begin)
> -                {
> -                    --pos;
> -                    if (pos->Contains(addr))
> -                    {
> -                        return &(*pos); 
> -                    }
> -                }
> +                    return &(*pos);
>             }
>             return NULL;
>         }
> @@ -1316,18 +1297,11 @@
>                 typename Collection::const_iterator end = m_entries.end();
>                 typename Collection::const_iterator pos = std::lower_bound (begin, end, range, BaseLessThan);
> 
> +                while(pos != begin && pos[-1].Contains(range))
> +                    --pos;
> +
>                 if (pos != end && pos->Contains(range))
> -                {
> -                    return &(*pos); 
> -                }
> -                else if (pos != begin)
> -                {
> -                    --pos;
> -                    if (pos->Contains(range))
> -                    {
> -                        return &(*pos); 
> -                    }
> -                }
> +                    return &(*pos);
>             }
>             return NULL;
>         }
> @@ -1501,12 +1475,15 @@
>                 typename Collection::iterator end = m_entries.end();
>                 typename Collection::iterator pos = std::lower_bound (begin, end, entry, BaseLessThan);
> 
> +                while(pos != begin && pos[-1].addr == addr)
> +                    --pos;
> +
>                 if (pos != end)
>                 {
>                     if (pos->addr == addr || !exact_match_only)
>                         return &(*pos);
>                 }
> -           }
> +            }
>             return NULL;
>         }
> <D1690.1.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits