[llvm] r346815 - Make dsymutil more robust when parsing load commands.

[Adrian Prantl via llvm-commits]

I don't see any mention of /tmp in the commit you replied to. There was a similar issue in clang recently that was fixed by r349083. Did you mean that one possibly?


The error you *linked to* is the one discussed in https://reviews.llvm.org/rL349066. It disappeared on other bots, so we didn't follow up. I'd be happy to debug it if I only had a symbolicated backtrace (which is unfortunately the error that we're seeing).

-- adrian

> On Dec 18, 2018, at 12:25 PM, Galina Kistanova <gkistanova at gmail.com> wrote:
> 
> Hello Adrian,
> 
> This commit has introduced a questionable dependency. The test depends on existence or not existence of the /tmp/t.o file, which is not a good idea.
> 
> The test could fail with some cryptic message if the file exists and is not what the test expects.
> Please see for example http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/22229/steps/test/logs/stdio.
> 
> Could you fix this, please?
> 
> Thanks
> 
> Galina
> 
> On Tue, Nov 13, 2018 at 3:33 PM Adrian Prantl via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> Author: adrian
> Date: Tue Nov 13 15:31:25 2018
> New Revision: 346815
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=346815&view=rev
> Log:
> Make dsymutil more robust when parsing load commands.
> 
> rdar://problem/45883463
> 
> Modified:
>     llvm/trunk/test/tools/dsymutil/Inputs/lc_build_version.x86_64
>     llvm/trunk/test/tools/dsymutil/X86/lc_build_version.test
>     llvm/trunk/tools/dsymutil/MachOUtils.cpp
> 
> Modified: llvm/trunk/test/tools/dsymutil/Inputs/lc_build_version.x86_64
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/Inputs/lc_build_version.x86_64?rev=346815&r1=346814&r2=346815&view=diff
> ==============================================================================
> Binary files llvm/trunk/test/tools/dsymutil/Inputs/lc_build_version.x86_64 (original) and llvm/trunk/test/tools/dsymutil/Inputs/lc_build_version.x86_64 Tue Nov 13 15:31:25 2018 differ
> 
> Modified: llvm/trunk/test/tools/dsymutil/X86/lc_build_version.test
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/lc_build_version.test?rev=346815&r1=346814&r2=346815&view=diff
> ==============================================================================
> --- llvm/trunk/test/tools/dsymutil/X86/lc_build_version.test (original)
> +++ llvm/trunk/test/tools/dsymutil/X86/lc_build_version.test Tue Nov 13 15:31:25 2018
> @@ -8,4 +8,4 @@ CHECK-NEXT:    platform:        1
>  CHECK-NEXT:    minos:           658944
>  CHECK-NEXT:    sdk:             658944
>  CHECK-NEXT:    ntools:          0
> -CHECK-NEXT: - cmd
> +CHECK-NEXT:   - cmd:             LC_BUILD_VERSION
> 
> Modified: llvm/trunk/tools/dsymutil/MachOUtils.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/MachOUtils.cpp?rev=346815&r1=346814&r2=346815&view=diff
> ==============================================================================
> --- llvm/trunk/tools/dsymutil/MachOUtils.cpp (original)
> +++ llvm/trunk/tools/dsymutil/MachOUtils.cpp Tue Nov 13 15:31:25 2018
> @@ -374,23 +374,28 @@ bool generateDsymCompanion(const DebugMa
> 
>    // Get LC_UUID and LC_BUILD_VERSION.
>    MachO::uuid_command UUIDCmd;
> -  MachO::build_version_command BuildVersionCmd;
> +  SmallVector<MachO::build_version_command, 2> BuildVersionCmd;
>    memset(&UUIDCmd, 0, sizeof(UUIDCmd));
> -  memset(&BuildVersionCmd, 0, sizeof(BuildVersionCmd));
>    for (auto &LCI : InputBinary.load_commands()) {
>      switch (LCI.C.cmd) {
>      case MachO::LC_UUID:
> +      if (UUIDCmd.cmd)
> +        return error("Binary contains more than one UUID");
>        UUIDCmd = InputBinary.getUuidCommand(LCI);
>        ++NumLoadCommands;
> -      LoadCommandSize += sizeof(MachO::uuid_command);
> +      LoadCommandSize += sizeof(UUIDCmd);
>        break;
> -    case MachO::LC_BUILD_VERSION:
> -      BuildVersionCmd = InputBinary.getBuildVersionLoadCommand(LCI);
> +   case MachO::LC_BUILD_VERSION: {
> +      MachO::build_version_command Cmd;
> +      memset(&Cmd, 0, sizeof(Cmd));
> +      Cmd = InputBinary.getBuildVersionLoadCommand(LCI);
>        ++NumLoadCommands;
> -      LoadCommandSize += sizeof(MachO::build_version_command);
> +      LoadCommandSize += sizeof(Cmd);
>        // LLDB doesn't care about the build tools for now.
> -      BuildVersionCmd.ntools = 0;
> +      Cmd.ntools = 0;
> +      BuildVersionCmd.push_back(Cmd);
>        break;
> +    }
>      default:
>        break;
>      }
> @@ -463,13 +468,13 @@ bool generateDsymCompanion(const DebugMa
>      OutFile.write(reinterpret_cast<const char *>(UUIDCmd.uuid), 16);
>      assert(OutFile.tell() == HeaderSize + sizeof(UUIDCmd));
>    }
> -  if (BuildVersionCmd.cmd != 0) {
> -    Writer.W.write<uint32_t>(BuildVersionCmd.cmd);
> -    Writer.W.write<uint32_t>(sizeof(BuildVersionCmd));
> -    Writer.W.write<uint32_t>(BuildVersionCmd.platform);
> -    Writer.W.write<uint32_t>(BuildVersionCmd.minos);
> -    Writer.W.write<uint32_t>(BuildVersionCmd.sdk);
> -    Writer.W.write<uint32_t>(BuildVersionCmd.ntools);
> +  for (auto Cmd : BuildVersionCmd) {
> +    Writer.W.write<uint32_t>(Cmd.cmd);
> +    Writer.W.write<uint32_t>(sizeof(Cmd));
> +    Writer.W.write<uint32_t>(Cmd.platform);
> +    Writer.W.write<uint32_t>(Cmd.minos);
> +    Writer.W.write<uint32_t>(Cmd.sdk);
> +    Writer.W.write<uint32_t>(Cmd.ntools);
>    }
> 
>    assert(SymtabCmd.cmd && "No symbol table.");
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits