Fix: flt.lttng-utils.debug-info: build id comparison
authorFrancis Deslauriers <francis.deslauriers@efficios.com>
Tue, 28 May 2019 21:40:53 +0000 (17:40 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Wed, 29 May 2019 16:38:29 +0000 (12:38 -0400)
commit9b1d6816c871c223111c817e33b23df01fd4dee1
treed62015b7cdef4ff981ac8c1e2133c1baef390416
parent19bdff207149e8d07bfa141901b887b8d979896c
Fix: flt.lttng-utils.debug-info: build id comparison

Issue
=====
Commit af54bd1801 fixed a endianness bug by coincidence.

The real bug is the following line:
  name_sz = (uint32_t) *buf;

The `buf` variable is a `uint8_t *` that points at a 32bits word storing
the name size field.

On that line we dereference `buf` and cast the result to a uint32_t.

Dereferencing `buf` gives a `uint8_t` containing the first byte of the
32bits word which is then cast to 32bits and stored in `name_sz`.
However, the intent was to read the full 32bits word in `name_sz`.

It was working properly on little-endian architectures because the first
byte of the 32bit word was probably always enough to contain the size of
the name so casting the `uint8_t` to a `uint32_t` did not lead to
truncating.

But on big-endian architectures, that same first byte of 32bits word
containing the name size is probably always zero which lead to a failure
to recognize the note as a build id note.

Wrong Solution
==============
Commit af54bd1801 assumed that the `elf_getdata()` returned the note
section in the file byte order and used `gelf_xlatetom()` to swap the by
to the machine byte order. This assumption is false, the `elf_getdata()`
man page says:
  The returned data descriptor will be setup to contain translated
  data[...]

So no need to translate from the file byte order to the machine byte
order after the extracting the data using `elf_getdata()`.

The mis-guided use of `gelf_xlatetom()` made the testcase pass on
PowerPC because it swapped the bytes of the 32bits word making the cast
described above work. But running that same test on a PowerPC on a
PowerPC binary would not swap the byte order (as machine and file byte
order are the same) and would trigger the casting bug described above.

Right Solution
==============
The right solution is to cast the `buf` pointer to a `uint32_t *` before
dereferencing it. Such as:
  name_sz = (uint32_t) *(uint32_t *)buf;

But, during this investigation, we discovered that the libelf library
offers the `gelf_getnote()` function that does all the work to extract
the note fields. Using this function makes it clearer and safer.

This commit uses this approach as well as reverting all the changes
added by commit af54bd1801.

Drawbacks
=========
None.

Reported-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I952aad69d225c202b3aa9c41724b6645550a2d68
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1340
Tested-by: jenkins
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
plugins/lttng-utils/debug-info/bin-info.c
plugins/lttng-utils/debug-info/bin-info.h
This page took 0.024957 seconds and 4 git commands to generate.