[PATCH] D96619: [LLD][COFF] PR49068: Include the IMAGE_REL_BASED_HIGHLOW relocation base type when the machine is 64 bits and the relocation type is ADDR32

Sat May 8 06:43:02 PDT 2021

aganea added a comment.

@ayrivera : Would you mind please giving a bit more context about your usage, and why do you need this to work?

I think this patch only partially solves the "exposed"/buggy part of the issue. A bit more would be needed if we want to be correct.

When linking your test .OBJ with MSVC link.exe, we get:

  >link simple.obj
  Microsoft (R) Incremental Linker Version 14.28.29914.0
  Copyright (C) Microsoft Corporation.  All rights reserved.

  simple.obj : error LNK2017: 'ADDR32' relocation to 'arr' invalid without /LARGEADDRESSAWARE:NO
  LINK : fatal error LNK1165: link failed because of fixup errors

The documentation for that error code indicates that there are several constraints when using 32-bit addressing in 64-bit images: https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-error-lnk2017?view=msvc-160

- For a starter, LLD does not display an equivalent error when linking `simple.obj` without the proper options.
- If using `/LARGEADDRESSAWARE:NO` like suggested by Microsoft, it seems LLD doesn't disable `/HIGHENTROPYVA` (it is still on)
- The doc seems to suggest to use `/FIXED` in this scenario, but `link.exe` seems happy without. However if the image is rellocated, the 32-bit addressing might not work?
- The default base when using /LARGEADDRESSAWARE:NO should probably be 0x400000, but LLD still generates 0x140000000
- Finally, the doc suggests to "limit" the image to 3 GB, but the PE/spec suggests the limit for a PE image size is already 2 GB: https://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/pecoff_v83.docx (section 3.4) so I think there's nothing to do for this.

I feel the above issues should be fixed/mitigated if we wanted to land this patch.

Subsidiary question, do we need to fix `llvm/lib/Object/RelocationResolver.cpp` as well? That is, add support for `IMAGE_REL_AMD64_ADDR32` in `supportsCOFFX86_64()` and `resolveCOFFX86_64()`?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96619/new/

https://reviews.llvm.org/D96619