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

Galina Kistanova via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 20 10:18:06 PST 2018


 I have blamed the correct commit.

The llvm/trunk/test/tools/dsymutil/Inputs/lc_build_version.x86_64 updated
by this commit contains a reference to /tmp/t.o.
To reproduce the problem locally simply put the attached t.o file to the
local /tmp directory and run the test. You can debug from there.
I guess, you just taken some random input file which happens to contain
more than you actually need for this test? Sanitizing that file would do
then.

Thanks

Galina


On Tue, Dec 18, 2018 at 1:00 PM Adrian Prantl <aprantl at apple.com> wrote:

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181220/d2f20d15/attachment.html>


More information about the llvm-commits mailing list