[PATCH] D39972: [LLD] [COFF] Support ENTRY and SUBSYSTEM in .drectve sections

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

I think I can approve if you add a test.



================
Comment at: COFF/Driver.cpp:249
+    case OPT_entry:
+      Config->Entry = addUndefined(mangle(Arg->getValue()));
+      break;
----------------
dmajor wrote:
> dmajor wrote:
> > mstorsjo wrote:
> > > ruiu wrote:
> > > > Are you really sure if you need `mangle`? IIRC, there are some command line options that behave differently in terms of name mangling when they are given via .drctve section, so I wonder if that is not the case for /entry.
> > > Also wrt `mangle`, it is possible that it also is done differently between gcc or clang in gnu mode, and msvc and clang in msvc mode. But if this is something that only is emitted e.g. with msvc specific pragmas that aren't enabled by default in gnu mode, it might not matter. (It does differ e.g. for mangling of symbol names in dllexport, which is supported in both modes.)
> > Sorry for the slow response. I finally got around to testing an I386 build (which is the only arch where this matters, according to the body of `mangle`) and I get an undefined symbol if I remove the call to `mangle`. So it seems that it's necessary.
> As far as I can tell (based on the comment above `PragmaCommentHandler::HandlePragma`) this is an msvc specific thing, so I wouldn't expect it to be enabled in gnu mode.
Cool! Thank you very much for investigating.


https://reviews.llvm.org/D39972