[ELF] FIx use-after-return of archive path

[ikudrin]

Summary

There is a bug in lld which on some occasions causes InputFile::ArchiveName to store incorrect value. This happens because library name allocated on stack then converted to StringRef and assigned to member variable.

[meadori]

Test case?

[ruiu]

I think a better approach would be to change the type of ArchiveName from StringRef to std::string. This patch may fix the issue, but I'm not 100% sure because it fixes only one path that can reach addFile function. There might be some other places at which we pass on-stack objects to the function.

[evgeny777]

Test case?

Any suggestions how to implement it?

[ikudrin]

I asked @evgeny777 and he let me take this revision.

[ikudrin]

• Change type of ArchiveName to std::string.
• Add a test.

[ikudrin]

• Remove "tee" in the test.

[ruiu]

LGTM

test/ELF/whole-archive-name.s

9

I think you can remove test/ELF/Inputs/whole-archive-name.s because you only need single .s file. Since --whole-archive links everything, you don't need to pass any .o file. I.e. you can just pass an .a file.

ELF/Driver.cpp

static DenseSet<StringRef> getExcludeLibs(opt::InputArgList &Args) {
   }
   return Ret;
 }
 
 static Optional<StringRef> getArchiveName(InputFile *File) {
   if (isa<ArchiveFile>(File))
     return File->getName();
   if (!File->ArchiveName.empty())
-    return File->ArchiveName;
+    return StringRef(File->ArchiveName);
   return None;
 }
 
 // Handles the -exclude-libs option. If a static library file is specified
 // by the -exclude-libs option, all public symbols from the archive become
 // private unless otherwise specified by version scripts or something.
 // A special library name "ALL" means all archive files.
 //

ELF/InputFiles.h

ArrayRef<Symbol *> getSymbols() {
     assert(FileKind == BinaryKind || FileKind == ObjKind ||
            FileKind == BitcodeKind || FileKind == ArchiveKind);
     return Symbols;
   }
 
   // Filename of .a which contained this file. If this file was
   // not in an archive file, it is the empty string. We use this
   // string for creating error messages.
-  StringRef ArchiveName;
+  std::string ArchiveName;
 
   // If this is an architecture-specific file, the following members
   // have ELF type (i.e. ELF{32,64}{LE,BE}) and target machine type.
   ELFKind EKind = ELFNoneKind;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint8_t OSABI = 0;
 
   // Cache for toString(). Only toString() should use this member.

ELF/InputSection.cpp

 std::string InputSectionBase::getObjMsg(uint64_t Off) {
   // Synthetic sections don't have input files.
   if (!File)
     return ("<internal>:(" + Name + "+0x" + utohexstr(Off) + ")").str();
   std::string Filename = File->getName();
 
   std::string Archive;
   if (!File->ArchiveName.empty())
-    Archive = (" in archive " + File->ArchiveName).str();
+    Archive = " in archive " + File->ArchiveName;
 
   // Find a symbol that encloses a given location.
   for (Symbol *B : File->getSymbols())
     if (auto *D = dyn_cast<Defined>(B))
       if (D->Section == this && D->Value <= Off && Off < D->Value + D->Size)
         return Filename + ":(" + toString(*D) + ")" + Archive;
 
   // If there's no symbol, print out the offset in the section.

test/ELF/whole-archive-name.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+// RUN: mkdir -p %t.dir
+// RUN: rm -f %t.dir/liba.a
+// RUN: llvm-ar rcs %t.dir/liba.a %t.o
+// RUN: ld.lld -L%t.dir --whole-archive -la -o %t -Map=- | FileCheck %s
+
+.globl _start
+_start:
+    nop
+
+// There was a use after free of an archive name.
+// Valgrind/asan would detect it.
+// CHECK:      liba.a(whole-archive-name.s.tmp.o):(.text)
+// CHECK-NEXT:     _start