[PATCH] D23552: [ELF] - Give automatically generated __start_* and __stop_* symbols default visibility.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 18 13:35:28 PDT 2016


ruiu added inline comments.

================
Comment at: ELF/SymbolTable.cpp:425
@@ -424,3 +424,3 @@
   std::tie(S, WasInserted) =
-      insert(N, STT_NOTYPE, STV_HIDDEN, /*CanOmitFromDynSym*/ false,
+      insert(N, STT_NOTYPE, StOther & 3, /*CanOmitFromDynSym*/ false,
              /*IsUsedInRegularObj*/ true, nullptr);
----------------
grimar wrote:
> ruiu wrote:
> > I'm not the type of person who wants to eliminate all constants, but 3? It's too magical.
> Actually it is used in many places in lld. I count 4 more places in this file (SymbolTable.cpp) and also once in Symbols.h:
> 
> ```
> uint8_t getVisibility() const { return StOther & 0x3; }
> ```
> 
> I suggest to leave it for now and fix once for all places if you want it.
The use of `0x3` in `getVisibility` is fine because it is obvious that it is extracting visibility bits. It actually hides the use of `0x3` and wraps it with a better name (getVisibility). But in this context there's no information as to what you are doing.


https://reviews.llvm.org/D23552





More information about the llvm-commits mailing list