Adding VERSION_MIN load command to MachO file on Darwin

Tue Aug 4 05:30:52 PDT 2015

Can some of the CHECKs be CHECK-NEXT?

Can you add an unversioned ios triple test?

LGTM with that.

Just in the future keep in mind that improving how we test things in
always in scope for some patch that needs that test. The ELF and COFF
raw dumpers were removed in r179362 (2013-04-12), but macho-dump is
still going :-(

On 30 July 2015 at 14:01, Steven Wu <stevenwu at apple.com> wrote:
> Ping.
>
> On Jul 24, 2015, at 11:10 AM, Steven Wu <stevenwu at apple.com> wrote:
>
> Attach updated patch. Rafael, does the new patch look OK now?
>
> Thanks
>
> Steven
>
> <0001-Force-the-MachO-generated-for-Darwin-to-have-VERSION.patch>
>
> On Jul 24, 2015, at 11:02 AM, Steven Wu <stevenwu at apple.com> wrote:
>
> Sure! Since I am currently doing the exact thing, I can’t argue with that!
>
> Steven
>
> On Jul 24, 2015, at 11:00 AM, Kevin Enderby <enderby at apple.com> wrote:
>
> Hi Steven,
>
> One nit, I would leave off the number “1” in "Load command 1” in your test.
> Just in case some day some other load command ends up first in the output.
>
> My thoughts,
> Kev
>
> On Jul 24, 2015, at 10:53 AM, Steven Wu <stevenwu at apple.com> wrote:
>
> Thanks Rafael and Kevin
>
> I will use llvm-objdump for now and the output is much readable for my
> tests. Whether adding the feature to dump macho load command to readobj is
> out of the scope for this problem and it will probably trigger more
> discussion. Here is the updated test:
>
> // RUN: llvm-mc -triple x86_64-apple-macosx10.10.0 %s -filetype=obj -o - |
> llvm-objdump -macho -private-headers - | FileCheck %s
> // RUN: llvm-mc -triple x86_64-apple-ios8.0.0 %s -filetype=obj -o - |
> llvm-objdump -macho -private-headers - | FileCheck %s
> --check-prefix=CHECK-IOS
> // RUN: llvm-mc -triple x86_64-apple-darwin %s -filetype=obj -o - |
> llvm-objdump -macho -private-headers - | FileCheck %s
> --check-prefix=CHECK-DARWIN
>
> // Test version-min load command should be inferred from triple and should
> always be generated on Darwin
> // CHECK: Load command 1
> // CHECK:       cmd LC_VERSION_MIN_MACOSX
> // CHECK:   cmdsize 16
> // CHECK:   version 10.10
>
> // CHECK-IOS: Load command 1
> // CHECK-IOS:       cmd LC_VERSION_MIN_IPHONEOS
> // CHECK-IOS:   cmdsize 16
> // CHECK-IOS:   version 8.0
>
> // CHECK-DARWIN-NOT: LC_VERSION_MIN
>
> Thanks
>
> Steven
>
> On Jul 24, 2015, at 10:42 AM, Kevin Enderby <enderby at apple.com> wrote:
>
> Hi Steven,
>
> You might consider using llvm-objdump in your test with the -macho and
> -private-headers options.  This will print the load commands as the darwin
> native otool(1) program prints them.
>
> Kev
>
> On Jul 24, 2015, at 9:59 AM, Steven Wu <stevenwu at apple.com> wrote:
>
> Hi Rafael
>
> I don’t see anyway to dump out MachO load commands using llvm-readobj. The
> feature is not implemented yet. I am happy to rewrite my test in the future
> when readobj is ready. There exists tests for assembly with directive
> already for iOS and OS X. This patch will not change their behavior. Update
> the patch to add iOS test and bare darwin macho tests.
>
> Thanks
>
> Steven
>
> Here is the updated test:
>
> // RUN: llvm-mc -triple x86_64-apple-macosx10.10.0 %s -filetype=obj -o - |
> macho-dump | FileCheck %s
> // RUN: llvm-mc -triple x86_64-apple-ios8.0.0 %s -filetype=obj -o - |
> macho-dump | FileCheck %s --check-prefix=CHECK-IOS
> // RUN: llvm-mc -triple x86_64-apple-darwin %s -filetype=obj -o - |
> macho-dump | FileCheck %s --check-prefix=CHECK-DARWIN
>
> // Test version-min load command should be inferred from triple and should
> always be generated on Darwin
> // CHECK:  (('command', 36)
> // CHECK:   ('size', 16)
> // CHECK:   ('version, 657920)
> // CHECK:   ('sdk, 0)
> // CHECK:  ),
>
> // CHECK-IOS:  (('command', 37)
> // CHECK-IOS:   ('size', 16)
> // CHECK-IOS:   ('version, 524288)
> // CHECK-IOS:   ('sdk, 0)
> // CHECK-IOS:  ),
>
>
> // CHECK-DARWIN: ('num_load_commands', 1)
> // CHECK-DARWIN-NOT: 'version'
>
>
> <0001-Force-the-MachO-generated-for-Darwin-to-have-VERSION.patch>
>
> On Jul 24, 2015, at 3:49 AM, Rafael Espíndola <rafael.espindola at gmail.com>
> wrote:
>
> Strange, Gmail displayed only the first part of the patch.
>
> Please don't use macho dump on new tests, readobj is far more readable.
>
> Please explicitly test what happens when there is a directive in the
> assembly. Also test with a ios, osx and bare metal mach-o triples.
>
> Cheers, Rafael
>
>
> On Jul 23, 2015, at 7:29 PM, Rafael Espíndola <rafael.espindola at gmail.com>
> wrote:
>
> Testcase?
>
> There is a test case in the patch itself
> (test/MC/MachO/darwin-version-min-load-command.s). And it is my intention to
> give every object file a VERSION_MIN load command. It is desired/required
> (depending on how strong you wish) to have this load command on darwin so
> the linker can prevent or warn you when linking in object files that are not
> compatible (for example, linking object built for 10.10 when you want to be
> 10.5 compatible or linking iPhone simulator objects into OS X binary).
>
> Thanks
>
> Steven
>
> In particular, is it the intention that an assembly file with no directives
> will get this load command?
>
> On Jul 23, 2015 9:51 PM, "Steven Wu" <stevenwu at apple.com> wrote:
>>
>> Hi Jim
>>
>> Here is a patch that forces all the MachO objects on Darwin to have
>> version_min load command. When MCObjectStreamer is created, it will inferred
>> the default version_min from target triple. When there is a version_min
>> directive in the assembly input, it will overwrite the default. By doing so,
>> it will not affect the assembly source that already have version_min
>> directive, but force a version_min load command on the others.
>>
>> The only relevant part of the change is following and the rest is just
>> updating the test for correct load command number and offset in the file:
>>
>> diff --git a/lib/MC/MCMachOStreamer.cpp b/lib/MC/MCMachOStreamer.cpp
>> index 53cd131..116ef09 100644
>> --- a/lib/MC/MCMachOStreamer.cpp
>> +++ b/lib/MC/MCMachOStreamer.cpp
>> @@ -493,6 +493,16 @@ MCStreamer *llvm::createMachOStreamer(MCContext
>> &Context, MCAsmBackend &MAB,
>>                                        bool LabelSections) {
>>    MCMachOStreamer *S = new MCMachOStreamer(Context, MAB, OS, CE,
>>                                             DWARFMustBeAtTheEnd,
>> LabelSections);
>> +  const Triple &TT = Context.getObjectFileInfo()->getTargetTriple();
>> +  if (TT.isOSDarwin()) {
>> +    unsigned Major, Minor, Update;
>> +    TT.getOSVersion(Major, Minor, Update);
>> +    // If there is a version specified, Major will be non-zero.
>> +    if (Major)
>> +      S->EmitVersionMin((TT.isMacOSX() ?
>> +                        MCVM_OSXVersionMin : MCVM_IOSVersionMin),
>> +                        Major, Minor, Update);
>> +  }
>>    if (RelaxAll)
>>      S->getAssembler().setRelaxAll(true);
>>    return S;
>>
>> Thanks
>>
>> Steven
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
>
>
>