New issue
Advanced search Search tips

Issue 892359 link

Starred by 1 user
Status: Fixed
Owner:
etiennep@chromium.org
Closed: Oct 11
Cc:
hua...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Zucchini: Discrepancy in reference width.

Project Member Reported by etiennep@chromium.org, Oct 4 
DisassemblerElfX64 returns a reference pool for abs that has width == 4.
However, Abs32RvaExtractorWin32 is used to extract abs references and assumes references have a WidthOf == 8.
This potentially causes 
CHECK(it == locations.begin() || offset - *(it - 1) >= addr.width());
to fail in Abs32RvaExtractorWin32.
I believe width should be 8 in both cases.

Comment 1 by huangs@google.com, Oct 5 

ELF should have its own Abs32RvaExtractor; or something refactoring should be done to create a more general class that's used by both. Note that it's possible for a 64-bit architecture to have abs32 references that are 4-bytes wide, e.g., AArch64.

Comment 2 by etiennep@chromium.org, Oct 5 

Both DisassemblerWin32 and DisassemblerElfIntel contain X86/X64 code sections with the same abs32 reference type. I think we should rename:
Abs32RvaExtractorWin32 -> Abs32RvaExtractorIntel
Abs32ReaderWin32 -> Abs32ReaderIntel
Then it's ok for disassemblers above to use those.
DisassemblerElfArm will use a different Abs32Reader class.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 5 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c

commit af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c
Author: Etienne Pierre-doray <etiennep@chromium.org>
Date: Fri Oct 05 20:15:13 2018

[Zucchini]: Fix discrepancy in reference width for Elf reloc and abs32.

Adds kVAWidth to Elf32Traits and use it in
DisassemblerElfIntel::MakeReferenceGroups as the width of
kReloc and kAbs32 reference types.

Bug:  892359 

Change-Id: I28930b8978393c16ee29051c48496e4f696a3fcd
Reviewed-on: https://chromium-review.googlesource.com/c/1264816
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597264}
[modify] https://crrev.com/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c/components/zucchini/abs32_utils.cc
[modify] https://crrev.com/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c/components/zucchini/abs32_utils.h
[modify] https://crrev.com/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c/components/zucchini/abs32_utils_unittest.cc
[modify] https://crrev.com/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c/components/zucchini/disassembler_elf.cc
[modify] https://crrev.com/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c/components/zucchini/disassembler_elf.h
[modify] https://crrev.com/af95efbfe9d3f3ca90c105f7d1c9c13e43fcde8c/components/zucchini/disassembler_win32.cc

Comment 4 by etiennep@chromium.org, Oct 11

Status: Fixed (was: Assigned)
►

Sign in to add a comment