[lld-macho] Make lld::getInteger() tolerate leading "0x"/"0X" when base is 16

[gkm]

Summary

ld64 is cool with leading 0x for hex command-line args, and we should be also.

[int3]

presumably this is related to D88064... could you reference it in the commit message and test the 0X case in D88064? (Actually, I've actually never seen 0X in the wild... does ld64 support it too?)

[int3]

nvm, I see all the hex values in D88064 aren't actually strings that we parse... but I'm still curious to know how this is going to be used / tested

[gkm]

I just added the rationale to the diff summary.

While inspecting ld64, I leaned that it uses strtoull(3) for parsing hex command-line args, and strtoull(3) does this: " If base is zero or 16, the string may then include a "0x" prefix, and the number will be read in base 16"

How will it be used? Somewhere in the wild, we will avoid bugs.

[gkm]

• add a test case

[int3]

Thanks for the explanation!

lld/test/MachO/headerpad.s

24

wouldn't hurt to test the 0X case too

[MaskRay]

lld/Common/Args.cpp

39

If base == 0, to_integer can parse the 0x prefix.

[gkm]

• add testcase for 0X prefix alongside 0x
• resequence diff stack so this can land now

[MaskRay]

??????

If base == 0, to_integer can parse the 0x prefix.

This is unaddressed. Please see StringRef.cpp GetAutoSenseRadix.

[gkm]

I only skip 0x when base == 16. My change does nothing to alter the base == 0 case, and GetAutoSenseRadix continues to operate as before.

[gkm]

lld/Common/Args.cpp

39

I am unclear about the intention of your statement. Are you suggesting that I change base to 0 and let to_integer() strip the 0x ?
I considered that, but that way we perform the 0x detection twice.

[MaskRay]

lld/Common/Args.cpp

39

If you use base 0, to_integer does not need a change and you get 0x ability for free.

[gkm]

lld/Common/Args.cpp

37–39

Is this what you prefer?

lld/Common/Args.cpp

 
 static int64_t getInteger(opt::InputArgList &args, unsigned key,
                           int64_t Default, unsigned base) {
   auto *a = args.getLastArg(key);
   if (!a)
     return Default;
 
   int64_t v;
-  if (to_integer(a->getValue(), v, base))
+  StringRef s = a->getValue();
+  if (base == 16 && (s.startswith("0x") || s.startswith("0X")))
+    s = s.drop_front(2);
+  if (to_integer(s, v, base))
     return v;
 
   StringRef spelling = args.getArgString(a->getIndex());
   error(spelling + ": number expected, but got '" + a->getValue() + "'");
   return 0;
 }
 
 int64_t lld::args::getInteger(opt::InputArgList &args, unsigned key,

lld/test/MachO/headerpad.s

 # PAD0:      sectname __text
 # PAD0-NEXT: segname __TEXT
 # PAD0-NEXT: addr
 # PAD0-NEXT: size
 # PAD0-NEXT: offset [[#%u, CMDSIZE + 32]]
 
 # RUN: lld -flavor darwinnew -o %t %t.o -headerpad 11
 # RUN: llvm-objdump --macho --all-headers %t | FileCheck %s --check-prefix=PAD11
-# PAD11:      magic        cputype  cpusubtype  caps    filetype ncmds sizeofcmds               flags
-# PAD11-NEXT: MH_MAGIC_64  X86_64   ALL         LIB64   EXECUTE  9     [[#%u, CMDSIZE:]] {{.*}}
+# RUN: lld -flavor darwinnew -o %t %t.o -headerpad 0x11
+# RUN: llvm-objdump --macho --all-headers %t | FileCheck %s --check-prefix=PAD11
+# RUN: lld -flavor darwinnew -o %t %t.o -headerpad 0X11
+# RUN: llvm-objdump --macho --all-headers %t | FileCheck %s --check-prefix=PAD11
+
+# PAD11:      magic        {{.+}}  ncmds sizeofcmds        flags
+# PAD11-NEXT: MH_MAGIC_64  {{.+}}  9     [[#%u, CMDSIZE:]] {{.*}}
 # PAD11:      sectname __text
 # PAD11-NEXT: segname __TEXT
 # PAD11-NEXT: addr
 # PAD11-NEXT: size
 # PAD11-NEXT: offset [[#%u, CMDSIZE + 32 + 0x11]]
 
 .globl _main
 _main:
   ret