Issue metadata
Sign in to add a comment
|
sys-power/dptf fails to build when SHT_RELR sections are enabled |
||||||||||||||||||||||
Issue descriptionI am testing support for SHT_RELR sections in chromium os. SHT_RELR sections are used to compactly store all the R_X86_64_RELATIVE relocations in a PIE binary or a shared object. For more background, see the official proposal here: https://groups.google.com/forum/#!topic/generic-abi/bX460iggiKg SHT_RELR sections do not support relative relocations with odd offsets. This results in sys-power/dptf failing to build when SHT_RELR sections are enabled. Steps to reproduce: 1. setup_board --board=chell 2. build_packages --board=chell 3. cherry pick https://chromium-review.googlesource.com/733802 4. sudo emerge cross-x86_64-cros-linux-gnu/binutils 5. emerge-chell sys-power/dptf fails with: usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.27.0/ld.gold.real: internal error in shrink_relocs, at output.h:2527 clang-6.0: error: linker command failed with exit code 1 (use -v to see invocation) when linking 'esif_ufd' binary. The relevant assert in gold linker is just checking that the offset is not odd: 2526 // Odd addresses are not supported in SHT_RELR. 2527 gold_assert(current%2 == 0);
,
Mar 9 2018
Digging deeper, the relocation containing odd offset is in the .../ESIF_UF/Sources/esif_uf_trace.o object.
It is pointing to the data segment. The corresponding global is defined in esif_uf_trace.c source file as:
struct esif_tracelevel_s g_traceinfo[] = {
{"FATAL", ESIF_TRACELEVEL_FATAL, ESIF_TRACEMASK_ALL, ESIF_TRACEROUTE_EVENTLOG},
{"ERROR", ESIF_TRACELEVEL_ERROR, ESIF_TRACEMASK_ALL, ESIF_TRACEROUTE_EVENTLOG},
{"WARNING", ESIF_TRACELEVEL_WARN, ESIF_TRACEMASK_ALL, ESIF_TRACEROUTE_DEBUGGER},
{"INFO", ESIF_TRACELEVEL_INFO, ESIF_TRACEMASK_ALL, ESIF_TRACEROUTE_DEBUGGER},
{"DEBUG", ESIF_TRACELEVEL_DEBUG, ESIF_TRACEMASK_ALL, ESIF_TRACEROUTE_DEBUGGER},
};
Further, struct esif_tracelevel_s is defined in esif_uf_trace.h as:
#pragma pack(push,1)
/* Trace Level Data */
struct esif_tracelevel_s {
const char * label; // label, i.e. "ERROR"
int level; // level, i.e, 1
esif_tracemask_t modules; // active module bitmask
esif_traceroute_t routes; // active routes bitmask
};
#pragma pack(pop)
And esif_tracemask_t/esif_traceroute_t are defined in the same file as:
/* Trace Module and Route Masks */
typedef u32 esif_tracemask_t;
typedef u8 esif_traceroute_t;
In the struct esif_tracelevel_s, the four fields are {8, 4, 4, 1} bytes, for a total of 17 bytes.
The #pragma pack(push,1) makes instances of this struct 1-byte aligned.
Thus, in the g_traceinfo[] global array of struct esif_tracelevel_s, the second entry starts exactly
17 bytes ahead of the first entry, ensuring that one of them will be starting at an odd address.
The relative relocation in question is pointing to the label field, which is the first field in the struct.
This is the source for odd offsets in the relocation table.
,
Mar 9 2018
Possible solutions I can think of: 1. Remove #pragma pack(push,1) from struct esif_tracelevel_s. Is this really necessary? As far as I can see, there is one array of this struct in esif_uf_trace.c containing only 5 entries. Removing the pragma increases the size of each entry in the array from 17 bytes to 24 bytes. 2. Change the pragma to specify 2 byte alignment with #pragma pack(push,2). This increases the size of each entry in the array from 17 bytes to 18 bytes, and ensures that each entry starts at an even address. 3. Implement support in gold linker for relative relocations with odd offsets. Theoretically, such relocations can be supported by simply excluding them from the .relr.dyn section (keeping them in the .rela.dyn section). I would like to avoid implementing (3), since odd offsets in relocations are pretty rare, and even when they do occur, like in this case, they seem to be accidental. It is better to fix the few odd cases resulting in odd offsets than to add general support for odd offsets in the linker.
,
Mar 9 2018
Thanks for the investigation. #1 should work. We will do some testing to make sure that traces still work as expected, then will upload a fix.
,
Mar 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/bdfbbc6ad2629c3a6eb8301e10407bbf5a628d7f commit bdfbbc6ad2629c3a6eb8301e10407bbf5a628d7f Author: Rahul Chaudhry <rahulchaudhry@chromium.org> Date: Fri Mar 16 04:23:28 2018 sys-power/dptf: append '--no-experimental-use-relr' to ldflags. This is the default behavior of gold linker right now, so this CL does not change any behavior. We are preparing for SHT_RELR sections to be enabled by default in gold. This change can be reverted once the real fix for http://crbug.com/820290 lands from upstream. BUG=chromium:820290 TEST='emerge-chell sys-power/dptf' works with SHT_RELR sections enabled in gold. Change-Id: I2afa7bdc28e0339e5b0355773311dec4d6b2a072 Reviewed-on: https://chromium-review.googlesource.com/965141 Commit-Ready: Rahul Chaudhry <rahulchaudhry@chromium.org> Tested-by: Rahul Chaudhry <rahulchaudhry@chromium.org> Reviewed-by: Manoj Gupta <manojgupta@chromium.org> Reviewed-by: Brian Bian <brian.bian@intel.com> [rename] https://crrev.com/bdfbbc6ad2629c3a6eb8301e10407bbf5a628d7f/sys-power/dptf/dptf-8.4.10400-r2.ebuild
,
Jun 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/5f3a23d0cc2858b5008d9fd343cbf8af818e8514 commit 5f3a23d0cc2858b5008d9fd343cbf8af818e8514 Author: Brian Bian <brian.bian@intel.com> Date: Wed Jun 06 04:28:39 2018 Update ebuild file for DPTF release 8.4.10401 Maintenance release with a few minor bug fixes: 1. Fixes to Critical Policy to enable the new UI (no functional changes). 2. Fix alignment issuse found in tracing ( http://crbug.com/920290 ). 3. Suppress sensor polling/trace messages when sensors are not enabled. BUG=chromium:820290 TEST=None Change-Id: I333cd3371041656312dc739754c6178d08b0e5b3 Signed-off-by: Brian Bian <brian.bian@intel.com> Reviewed-on: https://chromium-review.googlesource.com/1068216 Commit-Ready: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Commit-Ready: Brian Bian <brian.bian@intel.corp-partner.google.com> Tested-by: Brian Bian <brian.bian@intel.corp-partner.google.com> Reviewed-by: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Reviewed-by: Brian Bian <brian.bian@intel.corp-partner.google.com> Reviewed-by: Duncan Laurie <dlaurie@google.com> [rename] https://crrev.com/5f3a23d0cc2858b5008d9fd343cbf8af818e8514/sys-power/dptf/dptf-8.4.10401-r1.ebuild [modify] https://crrev.com/5f3a23d0cc2858b5008d9fd343cbf8af818e8514/sys-power/dptf/Manifest |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rahulchaudhry@chromium.org
, Mar 8 2018