This is an archive of the discontinued LLVM Phabricator instance.

[COFF] support /ignore:4217
ClosedPublic

Authored by inglorion on Dec 27 2017, 10:12 PM.

Details

Summary

lld-link accepts link.exe's /ignore option, but used to ignore
it. This can lead to semantic differences when warnings are treated as
fatal errors. One such case is when we resolve an __imp_ symbol to a
local definition. We emit a warning in that case, which /wx turns into
a fatal. This change makes lld-link accept /ignore:4217 to suppress
that warning, so that code that links with link.exe /wx /ignore:4217
links with lld-link, too.

Fixes PR35762.

Diff Detail

Repository
rL LLVM

Event Timeline

inglorion created this revision.

fix description of test

ruiu accepted this revision.Dec 27 2017, 10:47 PM

LGTM

lld/COFF/Driver.cpp
799–800 ↗(On Diff #128257)

nit: remove {}

lld/COFF/SymbolTable.cpp
120 ↗(On Diff #128257)

remove {}

133 ↗(On Diff #128257)

ditto

This revision is now accepted and ready to land.Dec 27 2017, 10:47 PM
inglorion marked 3 inline comments as done.

removed braces as per @ruiu's suggestions

This revision was automatically updated to reflect the committed changes.

General question: it looks like a lot of COFF tests are written using yaml2obj, whereas ELF always uses assembly files and llvm-mc (and I've been following that pattern for the few COFF patches I've written as well). Is there an actual preference for yaml2obj tests in COFF, or is it just a historic artifact? I personally find assembly tests a lot easier to understand.

rnk added a subscriber: zturner.Dec 28 2017, 9:07 AM

General question: it looks like a lot of COFF tests are written using yaml2obj, whereas ELF always uses assembly files and llvm-mc (and I've been following that pattern for the few COFF patches I've written as well). Is there an actual preference for yaml2obj tests in COFF, or is it just a historic artifact? I personally find assembly tests a lot easier to understand.

I agree, I'd recommend using .s files for all non-PDB tests. I use yaml mostly for objects with codeview because it @zturner added a great symbolic representation for codeview.

However, our COFF yaml isn't a very good representation for code and relocations. Code is represented as a binary blob and relocations are stored on the side. Storing relocations separately is a problem for codeview symbol records because if you want to add one record, you have to determine its size and update the offsets of all the relocations after the symbol. Type records don't have this problem because they do not contain relocations. Type merging is significantly more complicated than symbol linking, so we naturally have more tests for it, and increasing the testability of symbol records hasn't been as much of a priority.

The best way to unify these tests would probably be to add more directives to MC to emit complete codeview records. We can still test invalid records by falling back to open coding them with .byte, .short, and .long data directives.

General question: it looks like a lot of COFF tests are written using yaml2obj, whereas ELF always uses assembly files and llvm-mc (and I've been following that pattern for the few COFF patches I've written as well). Is there an actual preference for yaml2obj tests in COFF, or is it just a historic artifact? I personally find assembly tests a lot easier to understand.

I also agree that .s tests are easier. I remember some of the tests I've added recently where I despite that ended up using yaml2obj for features that weren't quite as easy to express in .s form.

General question: it looks like a lot of COFF tests are written using yaml2obj, whereas ELF always uses assembly files and llvm-mc (and I've been following that pattern for the few COFF patches I've written as well). Is there an actual preference for yaml2obj tests in COFF, or is it just a historic artifact? I personally find assembly tests a lot easier to understand.

I also agree that .s tests are easier. I remember some of the tests I've added recently where I despite that ended up using yaml2obj for features that weren't quite as easy to express in .s form.

Yup, there are definitely tests that aren't easily expressed (or aren't expressible at all) in assembly in COFF; I think ELF assembly directives are a bit richer (and clang's COFF assembly is still more expressive than MSVC's). I do think it's worthwhile to use them where they work though; for example, I believe the test in this file could be expressed as something like:

.globl foo
foo:
        ret

.globl main
main:
        jmp     *__imp_foo(%rip)