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

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<https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D60631&d=DwMGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ZRCyMsqk2PXhXzGVBp_E-J0TZOa4UT3Nhb3ohfmWi2U&s=lxOhNWDBz-2XPhpzp7IZFOfo-oViixs4vxGgVoDI6w4&e=>.
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190426/25cff0f6/attachment.html>

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

On Mon, Apr 29, 2019 at 3:25 AM Hans Wennborg <hans at chromium.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
>
It's hypothetically unbounded, of course, but I could see 50 being in
range, so 100 would probably be fool proof.

And yes, it seems reasonable to have a limit, but maybe a message when the
limit is a hit and a flag to override it would be an acceptable approach?

-- chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190503/306b56cf/attachment.html>

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>