[ELF] --warn-backrefs: use the same GroupId for object files in the same --{start,end}-lib

[ruiu]

Does --start-lib/--end-lib really implies --start-group/--end-group?

[MaskRay]

t3.o .globl bar; bar:
t4.o .globl foo; foo: call bar
t.o .global _start; _start: call bar

ld.gold t.o --start-lib t3.o t4.o --end-lib # successfully

Without this revision, ld.lld --warn-backrefs t.o --start-lib t3.o t4.o --end-lib warns that t4.o has a reference on t3.o.

[ruiu]

If that's the case, --start-lib implies --start-group. To handle it, I'd set InputFile::IsInGroup to true for --start-lib just like we are doing for --start-group, so that --start-lib is handled as if --start-group everywhere.

[MaskRay]

Treat --start-lib as --start-group when using --warn-backrefs

[MaskRay]

Updated.

gold disallows --start-group --start-lib nested in any order so we can simplify the handling.

// gold/options.cc

void
Input_arguments::start_group()
{
  if (this->in_group_)
    gold_fatal(_("May not nest groups"));
  if (this->in_lib_)
    gold_fatal(_("may not nest groups in libraries"));
  Input_file_group* group = new Input_file_group();
  this->input_argument_list_.push_back(Input_argument(group));
  this->in_group_ = true;
}

// Start a lib.

void
Input_arguments::start_lib(const Position_dependent_options& options)
{
  if (this->in_lib_)
    gold_fatal(_("may not nest libraries"));
  if (this->in_group_)
    gold_fatal(_("may not nest libraries in groups"));
  Input_file_lib* lib = new Input_file_lib(options);
  this->input_argument_list_.push_back(Input_argument(lib));
  this->in_lib_ = true;
}

[MaskRay]

Better error

[MaskRay]

Update test

[ruiu]

ELF/Driver.cpp

987

Do you need this? I think you now always set IsInGroup if you are InLib, so this new condition doesn't seem necessary.

[• espindola]

In D45849#1072825, @ruiu wrote:

Does --start-lib/--end-lib really implies --start-group/--end-group?

Not exactly. --start-lib/--end-lib means that those files are treated like a single .a. The tradition ELF handling of a .a file will loop over the members until no new member is fetched.

[MaskRay]

if (!InputFile::IsInGroup || InLib)
->
if (!InputFile::IsInGroup)

[• espindola]

LGTM

[saugustine]

This revision breaks build systems where the current directory isn't writable, because the test warn-backrefs.s just writes to a.out.

I have committed R330464 to add an output argument, which fixes the test.

ELF/Driver.cpp

case OPT_just_symbols:
       }
       break;
     case OPT_start_group:
       if (InputFile::IsInGroup)
         error("nested --start-group");
       InputFile::IsInGroup = true;
       break;
     case OPT_end_group:
       if (!InputFile::IsInGroup)
         error("stray --end-group");
       InputFile::IsInGroup = false;
       ++InputFile::NextGroupId;
       break;
     case OPT_start_lib:
+      if (InLib)
+        error("nested --start-lib");
+      if (InputFile::IsInGroup)
+        error("may not nest --start-lib in --start-group");
       InLib = true;
+      InputFile::IsInGroup = true;
       break;
     case OPT_end_lib:
       InLib = false;
+      InputFile::IsInGroup = false;
+      ++InputFile::NextGroupId;
       break;
     }
   }
 
   if (Files.empty() && errorCount() == 0)
     error("no input files");
 }
 

ELF/InputFiles.h

public:
 
   std::string getSrcMsg(const Symbol &Sym, InputSectionBase &Sec,
                         uint64_t Offset);
 
   // True if this is an argument for --just-symbols. Usually false.
   bool JustSymbols = false;
 
   // GroupId is used for --warn-backrefs which is an optional error
-  // checking feature. All files within the same --{start,end}-group
-  // get the same group ID. Otherwise, each file gets a new group
-  // ID. For more info, see checkDependency() in SymbolTable.cpp.
+  // checking feature. All files within the same --{start,end}-group or
+  // --{start,end}-lib get the same group ID. Otherwise, each file gets a new
+  // group ID. For more info, see checkDependency() in SymbolTable.cpp.
   uint32_t GroupId;
   static bool IsInGroup;
   static uint32_t NextGroupId;
 
 protected:
   InputFile(Kind K, MemoryBufferRef M);
   std::vector<InputSectionBase *> Sections;
   std::vector<Symbol *> Symbols;

test/ELF/start-lib.s

 // TEST2: Name: bar
 // TEST2-NOT: Name: foo
 
 // RUN: ld.lld -o %t3 %t1.o --start-lib %t2.o %t3.o
 // RUN: llvm-readobj --symbols %t3 | FileCheck --check-prefix=TEST3 %s
 // TEST3-NOT: Name: bar
 // TEST3-NOT: Name: foo
 
+// RUN: not ld.lld -o %t3 %t1.o --start-lib --start-lib %t2.o 2>&1 | FileCheck -check-prefix=NESTED-LIB %s
+// NESTED-LIB: nested --start-lib
+
+// RUN: not ld.lld -o %t3 %t1.o --start-group --start-lib %t2.o 2>&1 | FileCheck -check-prefix=LIB-IN-GROUP %s
+// LIB-IN-GROUP: may not nest --start-lib in --start-group
+
 .globl _start
 _start:

test/ELF/warn-backrefs.s

 # CHECK: backward reference detected: foo in {{.*}}1.o refers to {{.*}}2.a
 
 # RUN: not ld.lld --fatal-warnings --start-group --start-group 2>&1 | FileCheck -check-prefix=START %s
 # START: nested --start-group
 
 # RUN: not ld.lld --fatal-warnings --end-group 2>&1 | FileCheck -check-prefix=END %s
 # END: stray --end-group
 
+# RUN: echo ".globl bar; bar:" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t3.o
+# RUN: echo ".globl foo; foo: call bar" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t4.o
+# RUN: ld.lld --fatal-warnings --warn-backrefs %t1.o --start-lib %t3.o %t4.o --end-lib
+
 .globl _start, foo
 _start:
   call foo