[llvm] r269320 - [yaml2macho] Handle mach_header_64 reserved field

[Chris Bieneman via llvm-commits]

Good call. Revised in r271942.

-Chris

> On Jun 4, 2016, at 1:51 AM, Sean Silva <chisophugis at gmail.com> wrote:
> 
> 
> 
> On Thu, May 12, 2016 at 11:21 AM, Chris Bieneman via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: cbieneman
> Date: Thu May 12 13:21:09 2016
> New Revision: 269320
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=269320&view=rev <http://llvm.org/viewvc/llvm-project?rev=269320&view=rev>
> Log:
> [yaml2macho] Handle mach_header_64 reserved field
> 
> I've added the reserved field as an "optional" in YAML, but I've added asserts in the yaml2macho code to enforce that the field is present in mach_header_64, but not in mach_header.
> 
> Modified:
>     llvm/trunk/include/llvm/ObjectYAML/MachOYAML.h
>     llvm/trunk/lib/ObjectYAML/MachOYAML.cpp
>     llvm/trunk/test/ObjectYAML/MachO/mach_header_64.yaml
>     llvm/trunk/tools/yaml2obj/yaml2macho.cpp
> 
> Modified: llvm/trunk/include/llvm/ObjectYAML/MachOYAML.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ObjectYAML/MachOYAML.h?rev=269320&r1=269319&r2=269320&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ObjectYAML/MachOYAML.h?rev=269320&r1=269319&r2=269320&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/ObjectYAML/MachOYAML.h (original)
> +++ llvm/trunk/include/llvm/ObjectYAML/MachOYAML.h Thu May 12 13:21:09 2016
> @@ -30,7 +30,7 @@ struct FileHeader {
>    uint32_t ncmds;
>    uint32_t sizeofcmds;
>    llvm::yaml::Hex32 flags;
> -  // TODO: Need to handle the reserved field in mach_header_64
> +  llvm::yaml::Hex32 reserved;
>  };
> 
>  struct Object {
> 
> Modified: llvm/trunk/lib/ObjectYAML/MachOYAML.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/MachOYAML.cpp?rev=269320&r1=269319&r2=269320&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/MachOYAML.cpp?rev=269320&r1=269319&r2=269320&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/ObjectYAML/MachOYAML.cpp (original)
> +++ llvm/trunk/lib/ObjectYAML/MachOYAML.cpp Thu May 12 13:21:09 2016
> @@ -27,6 +27,8 @@ void MappingTraits<MachOYAML::FileHeader
>    IO.mapRequired("ncmds", FileHdr.ncmds);
>    IO.mapRequired("sizeofcmds", FileHdr.sizeofcmds);
>    IO.mapRequired("flags", FileHdr.flags);
> +  IO.mapOptional("reserved", FileHdr.reserved,
> +                 static_cast<llvm::yaml::Hex32>(0xDEADBEEFu));
>  }
> 
>  void MappingTraits<MachOYAML::Object>::mapping(IO &IO,
> 
> Modified: llvm/trunk/test/ObjectYAML/MachO/mach_header_64.yaml
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ObjectYAML/MachO/mach_header_64.yaml?rev=269320&r1=269319&r2=269320&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ObjectYAML/MachO/mach_header_64.yaml?rev=269320&r1=269319&r2=269320&view=diff>
> ==============================================================================
> --- llvm/trunk/test/ObjectYAML/MachO/mach_header_64.yaml (original)
> +++ llvm/trunk/test/ObjectYAML/MachO/mach_header_64.yaml Thu May 12 13:21:09 2016
> @@ -9,6 +9,7 @@ FileHeader:
>    ncmds:           0
>    sizeofcmds:      0
>    flags:           0x00218085
> +  reserved:        0x00000000
>  ...
> 
>  # CHECK: --- !mach-o
> @@ -20,4 +21,5 @@ FileHeader:
>  # CHECK:   ncmds:           0
>  # CHECK:   sizeofcmds:      0
>  # CHECK:   flags:           0x00218085
> +# CHECK:   reserved:        0x00000000
>  # CHECK: ...
> 
> Modified: llvm/trunk/tools/yaml2obj/yaml2macho.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/yaml2obj/yaml2macho.cpp?rev=269320&r1=269319&r2=269320&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/yaml2obj/yaml2macho.cpp?rev=269320&r1=269319&r2=269320&view=diff>
> ==============================================================================
> --- llvm/trunk/tools/yaml2obj/yaml2macho.cpp (original)
> +++ llvm/trunk/tools/yaml2obj/yaml2macho.cpp Thu May 12 13:21:09 2016
> @@ -30,6 +30,10 @@ public:
>                Obj.Header.magic == MachO::MH_CIGAM_64;
>      memset(reinterpret_cast<void *>(&Header64), 0,
>             sizeof(MachO::mach_header_64));
> +    assert((is64Bit || Obj.Header.reserved == 0xDEADBEEFu) &&
> +           "32-bit MachO has reserved in header");
> +    assert((!is64Bit || Obj.Header.reserved != 0xDEADBEEFu) &&
> +           "64-bit MachO has missing reserved in header");
>    }
> 
>    Error writeMachO(raw_ostream &OS);
> @@ -60,9 +64,11 @@ Error MachOWriter::writeHeader(raw_ostre
>    Header.ncmds = Obj.Header.ncmds;
>    Header.sizeofcmds = Obj.Header.sizeofcmds;
>    Header.flags = Obj.Header.flags;
> +  Header64.reserved = Obj.Header.reserved;
> 
> Why not just use a single Header64 object instead of the union? That avoids this dubious stuff where you're reading from the non-active part of the union below. I.e. you write into Header64, then try to read from Header after that without having all the fields of Header64.
> 
> -- Sean Silva
>  
> 
> -  if (is64Bit)
> +  if (is64Bit) {
>      OS.write((const char *)&Header64, sizeof(MachO::mach_header_64));
> +  }
>    else
>      OS.write((const char *)&Header, sizeof(MachO::mach_header));
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160606/94d1a4e6/attachment.html>