[PATCH] D45195: Add --check-library-dependency to maintain compatibility with other linkers

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 9 14:44:40 PDT 2018


MaskRay added inline comments.


================
Comment at: lld/ELF/Driver.cpp:934
+    case OPT_start_group:
+      ++InputFile::GroupDepth;
+      break;
----------------
ruiu wrote:
> grimar wrote:
> > I think `GroupDepth` should not be a member of `InputFile`.
> > It could be a local variable it seems:
> > 
> > ```
> >   int32_t GroupDepth = 0;
> >   for (auto *Arg : Args) {
> >     switch (Arg->getOption().getUnaliasedOption().getID()) {
> > ...
> >     case OPT_start_group:
> >       ++GroupDepth;
> >       break;
> >     case OPT_end_group:
> >       if (GroupDepth > 0)
> >         --GroupDepth;
> >       else
> >         error("stray --end-group");
> >       break;
> > ...
> >     }
> > 
> >     // All files within the same --start-group and --end-group get the
> >     // same group ID. Otherwise, a new file will get a new group ID.
> >     if (!GroupDepth)
> >       ++InputFile::CurrentGroupId;
> >   }
> > ```
> We can't do that because we have to handle linker scripts as well.
It seems gold does not support nested `--start-group --end-group`

```
% ld.gold -\( -\( a.o -\) -\)     
ld.gold: fatal error: May not nest groups
```


================
Comment at: lld/ELF/SymbolTable.cpp:336
+//
+//   ld.lld A B --start-group C D --end-group E
+//
----------------
Does `A` forms group 0 while `B` forms group 1?


https://reviews.llvm.org/D45195





More information about the llvm-commits mailing list