[PATCH] [ELF] Initial support for symbols visibility

Shankar Kalpathi Easwaran shankarke at gmail.com
Sat Mar 14 15:54:40 PDT 2015


Forgot to mention about your previous change, that you need to  add changes to ReaderWriterYAML(YAML Testing) and ReaderNative and WriterNative(Native formats) too.

Adding other reviewers here.


================
Comment at: lib/ReaderWriter/ELF/SectionChunks.h:790
@@ -776,1 +789,3 @@
+  }
+  sym.setVisibility(visibility);
   sym.setBindingAndType(binding, type);
----------------
This is very confusing, if you see line 771. We set binding for local symbols to be STB_LOCAL if scope is set to scopeTranslationUnit, so visibility would need to be set as STV_DEFAULT ? No ?





================
Comment at: lib/ReaderWriter/ELF/SectionChunks.h:804
@@ -787,2 +803,3 @@
+    // the other bits of st_other.
     sym.st_other = llvm::ELF::STV_HIDDEN;
     binding = llvm::ELF::STB_LOCAL;
----------------
could this be sym.st_other = sym.st_other & ~0x3 |llvm::ELF::STV_HIDDEN ?

================
Comment at: lib/ReaderWriter/ELF/SectionChunks.h:814
@@ -796,1 +813,3 @@
+  case AbsoluteAtom::scopeProtected:
+    break;
   }
----------------
assert

http://reviews.llvm.org/D8325

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list