[PATCH] D58739: [COFF] Add address-taken import thunks to the fid table

Wed Feb 27 16:38:58 PST 2019

rnk marked an inline comment as done.
rnk added inline comments.

================
Comment at: lld/test/COFF/guardcf-thunk.s:28
+        .section .text,"rx"
+        .def     main; .scl    2; .type   32; .endef
+        .global main
----------------
rnk wrote:
> dmajor wrote:
> > rnk wrote:
> > > I think `IMAGE_SYM_DTYPE_FUNCTION` is set by the .type directive in this .def/.endef block. I guess what we're doing doesn't match MSVC, but it also seems reasonable. Is the Rust function written in assembly, or is it emitted in LLVM? We should be setting the appropriate type...
> > The function is `on_tls_callback` at https://github.com/rust-lang/rust/blob/master/src/libstd/sys/windows/thread_local.rs#L201.
> > 
> > (It's unclear to me if the linker magic related to TLS callbacks is related to this issue. This is just the first function we fail at. I don't know if there are more "normal" functions that we'd fail on after it.)
> Well, most global variables also seem to have type "notype", so the only way we could make this work is by dropping the symbol type check completely and just looking at the section executability.
I checked, and MSVC performs this symbol type check, so we are being compatible, and I don't think we should remove it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58739