[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