Adding VERSION_MIN load command to MachO file on Darwin
Steven Wu
stevenwu at apple.com
Wed Aug 5 08:42:40 PDT 2015
I addressed your review and committed in r244059.
Thanks
Steven
> On Aug 4, 2015, at 5:30 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>
> 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
>>
>>
>>
>>
>>
>>
>>
More information about the llvm-commits
mailing list