[PATCH] D117933: [ELF] Don't consider directories of the same name as libraries

ben via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 24 15:59:59 PST 2022


bd1976llvm added a comment.

In D117933#3267645 <https://reviews.llvm.org/D117933#3267645>, @MaskRay wrote:

> In D117933#3267519 <https://reviews.llvm.org/D117933#3267519>, @mcgrathr wrote:
>
>> In D117933#3267443 <https://reviews.llvm.org/D117933#3267443>, @MaskRay wrote:
>>
>>> Currently a deplib name (`${name}`) indicates either the literal `${name}` or the -l style `lib${name}.{a,so}`.
>>> We have a conflict if `${name}` is `foo` and there is a directory named `foo`.
>>>
>>> What if we do
>>>
>>>   if (name.endswith(".a") || name.endswith(".so"))
>>>     findFromSearchPaths
>>>   else
>>>     searchLibraryBaseName
>>>
>>> ?
>>>
>>> That should avoid the conflict, too. A directory is unlikely named `.so` or `.a`.
>>
>> I don't think introducing more magical rules about naming patterns ever makes things easier.
>>
>> It is surprising at first blush that deplibs searches for "foo" and not just what "-lfoo" would do, but it makes sense if you want to allow saying "libfoo.a" in deplibs to avoid matching "libfoo.so" in arcane cases.
>> So I think just the fix here to make all cases of not-existing-files candidates on search paths be ignored robustly is sufficient.  The extra set of candidates that deplibs cases will attempt is tolerable and IMHO it's far preferable to having any more magic in the semantics of the behavior than there already is
>
> It's not adding more magical rules. The chained findFromSearchPaths and searchLibraryBaseName is replaced with testing just one form.
> We can then get rid of `llvm::function_ref<bool(StringRef)>`.

I would rather keep the search scheme as simple and general as possible. The ".a" and ".so" suffixes are just conventions (not part of the ELF standard). My hope was to make the autolinking scheme implementable by any ELF linker.

LGTM to the proposed fix from me. If memory serves, I experimented with the same approach (as proposed here) when autolinking was first implemented; however, I decided to make the code as minimal as possible and see what fell out from real world usage as It wasn't clear to me what problems build systems would be able to workaround vs which would be blockers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117933/new/

https://reviews.llvm.org/D117933



More information about the llvm-commits mailing list