[lld-macho] Silently ignore the -objc_abi_version

[keith]

Summary

This undocumented ld64 flag, based on the most recent ld64 source dump
from Xcode 12, only applies to i386. It seems like on all newer
architectures this behavior is the default.

[keith]

This one is a bit of a stretch, I'm not sure if you want to start dropping options like this, especially since Apple reserves the right to change the behavior over time. The only benefit in ignoring this is without dropping it, if you want to maintain compatibility and users are passing this, they cannot eliminate the warning produced (or the failure if they're using -fatal_warnings)

[oontvoo]

lld/MachO/Options.td

1324

Why do we need this? Doesn't removing the Flags<HelpHidden> effectively ignore it silently?

(should also add a test to silent-ignore.test)

[keith]

lld/MachO/Options.td

1324

This is primarily for the C++ change that stops this category from printing a warning. That's what silently means in this case.

[int3]

I like this, but let's see if anyone else chimes in over the next few days. I agree that we should add it to silent-ignore.test though.

[keith]

In D113070#3104810, @int3 wrote:

I like this, but let's see if anyone else chimes in over the next few days. I agree that we should add it to silent-ignore.test though.

I just took a list at this test, it seems like it's a bit invalid today. It looks like --version short circuits too early and the invalid flags aren't a problem in that case. -objc_abi_version is actually already in there. Am I reading the recommendation right there? I can update it to actually try to link a file so it gets far enough to error if that's the intent?

[int3]

Oh, I remember what that test was checking for now... it was just there to make sure that any unimplemented flags were still being parsed correctly, regardless of whether we were emitting diagnostic warnings/errors for them. (Our Options.td initially had a bunch of flags expecting wrong numbers of arguments, which messed up the parsing of all the other flags even when they were ignored. That's the error condition that the --not-an-ignored-argument run command is checking for.)

I think for this diff, we can add another RUN command to the silent-ignore.test, but this time specifically checking that we don't emit any diagnostics for -objc_abi_version.

[keith]

Add test making sure flag doesn't warn

[int3]

some additional comments would be good, but otherwise lgtm. Thanks!

lld/test/MachO/silent-ignore.s

4

19

[keith]

Add comments to test

lld/MachO/Driver.cpp

case OPT_grp_undocumented:
     break;
   case OPT_grp_obsolete:
     warn("Option `" + opt.getPrefixedName() +
          "' is obsolete. Please modernize your usage.");
     break;
   case OPT_grp_ignored:
     warn("Option `" + opt.getPrefixedName() + "' is ignored.");
     break;
+  case OPT_grp_ignored_silently:
+    break;
   default:
     warn("Option `" + opt.getPrefixedName() +
          "' is not yet implemented. Stay tuned...");
     break;
   }
 }
 
 static const char *getReproduceOption(InputArgList &args) {

lld/MachO/Options.td

 def no_compact_unwind : Flag<["-"], "no_compact_unwind">,
     HelpText<"This option is undocumented in ld64">,
     Flags<[HelpHidden]>,
     Group<grp_undocumented>;
 def no_new_main : Flag<["-"], "no_new_main">,
     HelpText<"This option is undocumented in ld64">,
     Flags<[HelpHidden]>,
     Group<grp_undocumented>;
-def objc_abi_version : Separate<["-"], "objc_abi_version">,
-    HelpText<"This option is undocumented in ld64">,
-    Flags<[HelpHidden]>,
-    Group<grp_undocumented>;
 def pause : Flag<["-"], "pause">,
     HelpText<"This option is undocumented in ld64">,
     Flags<[HelpHidden]>,
     Group<grp_undocumented>;
 def random_uuid : Flag<["-"], "random_uuid">,
     HelpText<"This option is undocumented in ld64">,
     Flags<[HelpHidden]>,
     Group<grp_undocumented>;

 def M : Flag<["-"], "M">,
     HelpText<"This option is ignored in ld64">,
     Flags<[HelpHidden]>,
     Group<grp_ignored>;
 def new_linker : Flag<["-"], "new_linker">,
     HelpText<"This option is ignored in ld64">,
     Flags<[HelpHidden]>,
     Group<grp_ignored>;
+
+def grp_ignored_silently : OptionGroup<"ignored_silently">, HelpText<"IGNORED SILENTLY">;
+
+def objc_abi_version : Separate<["-"], "objc_abi_version">,
+    HelpText<"This option only applies to i386 in ld64">,
+    Flags<[HelpHidden]>,
+    Group<grp_ignored_silently>;

lld/test/MachO/silent-ignore.s

+# REQUIRES: x86
+
+## Check that we correctly parse these flags, even though they are
+## unimplemented. ## We may still emit warnings or errors for some of the
+## unimplemented ones (but those ## errors are squelched because of the
+## `--version` flag.)
+# RUN: %lld --version \
+# RUN:   -dynamic \
+# RUN:   -no_deduplicate \
+# RUN:   -lto_library /lib/foo \
+# RUN:   -macosx_version_min 0 \
+# RUN:   -no_dtrace_dof \
+# RUN:   -dependency_info /path/to/dependency_info.dat \
+# RUN:   -lto_library ../lib/libLTO.dylib \
+# RUN:   -mllvm -time-passes \
+# RUN:   -objc_abi_version 2 \
+# RUN:   -ios_simulator_version_min 9.0.0 \
+# RUN:   -sdk_version 13.2
+# RUN: not %lld -v --not-an-ignored-argument 2>&1 | FileCheck %s
+# CHECK: error: unknown argument '--not-an-ignored-argument'
+
+## Check that we don't emit any warnings nor errors for these unimplemented flags.
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: %lld %t.o -o /dev/null -objc_abi_version 2
+
+.globl _main
+_main:
+  ret

lld/test/MachO/silent-ignore.test

-RUN: %lld --version \
-RUN:   -dynamic \
-RUN:   -no_deduplicate \
-RUN:   -lto_library /lib/foo \
-RUN:   -macosx_version_min 0 \
-RUN:   -no_dtrace_dof \
-RUN:   -dependency_info /path/to/dependency_info.dat \
-RUN:   -lto_library ../lib/libLTO.dylib \
-RUN:   -mllvm -time-passes \
-RUN:   -objc_abi_version 2 \
-RUN:   -ios_simulator_version_min 9.0.0 \
-RUN:   -sdk_version 13.2
-RUN: not %lld -v --not-an-ignored-argument 2>&1 | FileCheck %s
-CHECK: error: unknown argument '--not-an-ignored-argument'