[PATCH] D126046: [lld-macho] Support -non_global_symbols_strip_list, -non_global_symbols_no_strip_list, -x

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo marked an inline comment as done.
oontvoo added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1416-1417
+  // both be present.
+  // But -x can be used with either of these two, in which case, the last arg
+  // takes effect.
+  // (TODO: This is kind of confusing - considering disallowing using them
----------------
thevinster wrote:
> Just to check here, does ld64 also do the same here? Wondering if this specific to our implementation or inheriting behavior for ld64. I do agree it's confusing and maybe we should disallow it if no one actually does this in practice. 
Yes, that's from LD64. People may not do it on purpose but perhaps generated link commands do this. 

I'd slightly prefer we error-ing out for simplicity but given the behaviour is not too complex to imitate, I don't feel strongly about it, hence the TODO note. 

Does anyone else have a strong opinion on this? Otherwise, happy to just delete the TODO and be done with this edge case :)


================
Comment at: lld/test/MachO/local-symbol-output.s:73
+
+## Test interactions with exported_symbol.
+# RUN: %lld %t/main.o -o %t/strip_all_export_one.out \
----------------
thevinster wrote:
> oontvoo wrote:
> > thevinster wrote:
> > > Can we also have a test for interactions with `-unexported_symbol` as well? Specifically, that a global doesn't get included in the symbol table?
> > I'm not sure I understand the test you're proposing. Can you give an example?
> >  Specifically, -x and -non_global_symbols_[no_]strip_list options don't have an effect on global symbols. So how would using them with -unexported_symbol be meaningful?
> Right, -x and -non_global_symbols_[no_]strip_list don't matter to global symbols but unexported symbols can make them non-global. For example,
> ```
> .globl _foo
> .globl _bar
> _foo:
>   ret
> _bar:
>   ret
> ```
> What I'm thinking is if `_foo` in the test case above is unexported with -x, I would to expect to see only _bar in the symbol table since -x would've stripped away _foo. 
> 
> We can also apply this to -non_global_symbols_strip_list  and non_global_symbols_no_strip_list well by checking that the unexported symbol should've been stripped and not stripped from the symbol table, respectively. 
added tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126046