On 02/23/2012 09:37 AM, Maynard Johnson wrote:
> The problem reported here was originally reported in June 2009. The simple
> patch (below) that I proposed at the time was not accepted. Instead, the
> decision was made to try to solve the problem using a SEC_LOAD filter
> in bfd_support::interesting_symbol(). As it turns out, that was a bad decision
> which we proceeded to make worse by doubling down with the translate_debuginfo_syms
> function. You can see more details in the list posting with the subject of
> "Re: [PATCH] Fix debuginfo file processing so we don't require section vmas to match".
> The last posting on this thread was yesterday. The upshot is that I've already
> committed a fix to revert the ill-conceived changes we made in the past. And now
> this patch below is being re-submitted to fix the original problem. If I
> receive no comments by this time next week, I'll commit this fix.
> A user reported the following error:
> opreport error: profile_t::samples_range(): start > end something wrong
> with kernel or module layout ?
> please report problem to oprofile-list@...
> I discovered a bug in op_bfd:symbol_size() that was causing this. I found
> that the symbol was apparently in the last section, so the code fell through
> to the "else" clause, so end was being set to "file_size". In the case at
> hand, the sym filepos value (i.e., "start") was 0x19ee8 and file_size was
> 0x14bca; thus, start > end. This particular piece of code exists for the
> corner case of last symbol in the last section. Using 'file_size' as the
> default was, I presume, just a SWAG anyway. The attached patch just ensures
> that SWAG will always provide an answer that is logically correct.
> The binary being profiled was built with xlC under the old ppc ABI. The
> problematic symbol was in the BSS PLT section. While use of the old BSS PLT
> is discouraged (becuase it opens a vulnerability to buffer overrun exploits),
> it's still "valid" code. The reason opreport failed was that the symbol value
> plus the section filepos was greater than file size, which messed up our
> calculation of symbol size. For this particular program, BSS (which will be
> initialized by the dynamic linker at run time) is large enough so that it does
> extend past the size of the binary file. A symbol referencing PLT or BSS
> storage will, then, have a value that is greater than file_size.
> This patch makes a simple change to the symbol_size() function in how
> the 'end' value is set for the corner case of the "last symbol in the
> last section". The existing code for this corner case is a hack -- and
> one that doesn't always work, as was seen from this bug report. This
> patch just adds a bit more padding to the hack to make sure that 'end'
> will always be greater than 'start'.
> Signed-off-by: Maynard Johnson <maynardj@...>
> libutil++/op_bfd.cpp | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> diff --git a/libutil++/op_bfd.cpp b/libutil++/op_bfd.cpp
> index d03a99d..23a91b6 100644
> --- a/libutil++/op_bfd.cpp
> +++ b/libutil++/op_bfd.cpp
> @@ -394,7 +394,7 @@ size_t op_bfd::symbol_size(op_bfd_symbol const & sym,
> if (next && (sym.section() != next->section()))
> end = sym.symbol_endpos();
> - end = next ? next->filepos() : file_size;
> + end = next ? next->filepos() : sym.section()->filepos + file_size;
> if (start > end)
> return 0;