[PATCH] D133267: [Verifier] Reject dllexport with non-default visibility

[ben via Phabricator via llvm-commits]

bd1976llvm added a comment.



In D133267#3773235 <https://reviews.llvm.org/D133267#3773235>, @MaskRay wrote:

> In D133267#3771894 <https://reviews.llvm.org/D133267#3771894>, @bd1976llvm wrote:
>
>> Hi!  Sorry for not raising this issue prior to commit. The dllimport restriction seems correct to us. However, in PlayStation we are using dllexportclass=dllexport visibility=protected GlobalValues in LLVM IR. The protected visibility indicates that the symbol cannot be preempted and could be exported and dllexport indicates that it should be exported. This allows for implementing export control from C/C++ source code. This change has made this usage illegal. It doesn't makes sense semantically to restrict dllexport to visibility=default in LLVM IR, the restriction should be to visibility != hidden as hidden symbols cannot be exported by ELF linkers. Thanks.
>
> Oh, didn't know. Pushed 97d00b72a2b0a7aca631e1402a647f32c4e8bafb <https://reviews.llvm.org/rG97d00b72a2b0a7aca631e1402a647f32c4e8bafb> to allow `dllexport protected`.
> Does PlayStation need to relax the clang check D133266 <https://reviews.llvm.org/D133266>, too? (I have noticed that this combo makes sense but rejected it for simplicity.)

Thanks, much appreciated :) I think we should relax the D133266 <https://reviews.llvm.org/D133266> as well for completeness and to prevent customers having to edit their source code when it worked with previous compilers. However, I have spotted a potential problem with D133266 <https://reviews.llvm.org/D133266>. I will add a comment on that review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133267