llvm dev - May 2019 - Total response file count limited to 21

IMO, a limit of at most 20 nested response files would make a lot more
sense than 20 total response files. I don't think the total should really
have a limit at all.

Since we expand the files in place while iterating over the arglist, we'd
need to keep a separate array listing the end-offset of each file we're
currently nested within, and update the offsets with every expansion of
arguments. That's a bit more complex than just an integer count of number
of files seen so far, but it should be implementable completely within the
ExpandResponseFiles function, and I don't think it'd be _that_ tricky.


On Mon, Apr 29, 2019 at 3:26 AM Hans Wennborg via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> It seems reasonable to have an expansion limit, but maybe 20 is too low.
>
> Chris: You say you're relying heavily on nested response files. Do you
> have a feel for what would be a reasonable limit that wouldn't
> interfere with your use case?
>
> Thanks,
> Hans
>
> On Fri, Apr 26, 2019 at 5:37 PM Shoaib Meenai <smeenai at fb.com>
wrote:
> >
> > Actually, sorry, my fix was for the case where you had other arguments
> beginning with @ that weren't response files, whereas yours has actual
> response files, so my patch won't help there. CCing Reid and Hans, who
did
> a bunch of the implementation in this area.
> >
> >
> >
> > From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Shoaib
> Meenai via llvm-dev <llvm-dev at lists.llvm.org>
> > Reply-To: Shoaib Meenai <smeenai at fb.com>
> > Date: Friday, April 26, 2019 at 8:34 AM
> > To: Chris Glover <chrisglover at google.com>, "llvm-dev at
lists.llvm.org" <
> llvm-dev at lists.llvm.org>
> > Subject: Re: [llvm-dev] Total response file count limited to 21
> >
> >
> >
> > Hi Chris,
> >
> >
> >
> > I fixed this in https://reviews.llvm.org/D60631. If you're using a
> released version of clang you'll have to wait for 9.0 though.
> >
> >
> >
> > Thanks,
> >
> > Shoaib
> >
> >
> >
> > From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Chris
> Glover via llvm-dev <llvm-dev at lists.llvm.org>
> > Reply-To: Chris Glover <chrisglover at google.com>
> > Date: Friday, April 26, 2019 at 8:13 AM
> > To: "llvm-dev at lists.llvm.org" <llvm-dev at
lists.llvm.org>
> > Subject: [llvm-dev] Total response file count limited to 21
> >
> >
> >
> > Hi,
> >
> >
> >
> > I recently hit this on a project using a build system that relies
> heavily on nested response files. We found we could only have 21 response
> files total before getting errors related to the unexpanded response files.
> I tracked it down to this code in llvm/lib/Support/CommandLine.cpp
> >
> >
> >
> >     // If we have too many response files, leave some unexpanded. 
This
> avoids
> >     // crashing on self-referential response files.
> >     if (RspFiles++ > 20)
> >       return false;
> >
> >
> >
> > This seems rather arbitrary and in tests I was able to increase it to
> 200 reliably, which we could do locally for now, but I feel there must be a
> better way to handle this by tracking processed response files instead of
> just bailing like this. Or am I missing something?
> >
> >
> >
> > Thanks!
> >
> >
> >
> > -- chris
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190503/8286f9dd/attachment.html>

On Fri, May 3, 2019 at 1:30 PM James Y Knight via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> IMO, a limit of at most 20 nested response files would make a lot more
> sense than 20 total response files. I don't think the total should
really
> have a limit at all.
>
> Since we expand the files in place while iterating over the arglist,
we'd
> need to keep a separate array listing the end-offset of each file we're
> currently nested within, and update the offsets with every expansion of
> arguments. That's a bit more complex than just an integer count of
number
> of files seen so far, but it should be implementable completely within the
> ExpandResponseFiles function, and I don't think it'd be _that_
tricky.
>
>
Seems reasonable to me. And would absolutely solve the issue here (I think
we only nest 2 or 3 levels deep).

 If this is an agreed upon solution, I'd be happy to attempt to implement
it.

-- chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190506/7e1e1bbd/attachment.html>

Instead of bumping up the threshold, can we detect a cycle in reference
files? We have `llvm::sys::fs::equivalent` to determine whether two file
descriptors are for the same file or not. It doesn't seem too hard to
detect a cycle using that function.

*From: *Chris Glover via llvm-dev <llvm-dev at lists.llvm.org>
*Date: *Tue, May 7, 2019 at 6:04 AM
*To: *James Y Knight
*Cc: *llvm-dev at lists.llvm.org

On Fri, May 3, 2019 at 1:30 PM James Y Knight via llvm-dev
<
> llvm-dev at lists.llvm.org> wrote:
>
>> IMO, a limit of at most 20 nested response files would make a lot more
>> sense than 20 total response files. I don't think the total should
really
>> have a limit at all.
>>
>> Since we expand the files in place while iterating over the arglist,
we'd
>> need to keep a separate array listing the end-offset of each file
we're
>> currently nested within, and update the offsets with every expansion of
>> arguments. That's a bit more complex than just an integer count of
number
>> of files seen so far, but it should be implementable completely within
the
>> ExpandResponseFiles function, and I don't think it'd be _that_
tricky.
>>
>>
> Seems reasonable to me. And would absolutely solve the issue here (I think
> we only nest 2 or 3 levels deep).
>
>  If this is an agreed upon solution, I'd be happy to attempt to
implement
> it.
>
> -- chris
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190507/9221bd26/attachment.html>