Endless operator-> chain causing infinite loop

[Arthur O'Dwyer]

IMHO, it would be better to remove the command-line option, and just
set the (hard-coded) limit to 2048 or something else of that general
order of magnitude. I can't think of any valid use for deeply nested
operator->s; it's not like recursive template instantiation, where you
might want to use a higher limit to implement a useful behavior.
Therefore, setting the limit to something in the thousands ought to be
good enough until someone legitimately runs into the limit; and in the
meantime, we're not polluting the command-line docs with obscure and
useless options.

Also FWIW, I do think a DR ought to be opened to make this an explicit
implementation limit in the Standard... but I'm not volunteering. :)

my $.02,
–Arthur


On Fri, Oct 25, 2013 at 7:45 AM, Rahul Jain <1989.rahuljain at gmail.com> wrote:
>
> Hi,
>
> This is with reference to the below discussion thread:
>
> http://clang-developers.42468.n3.nabble.com/Endless-operator-gt-chain-causing-infinite-loop-td4035258.html
>
> I have tried to implement a command line argument to handle the issue.
>
> The patch for the same is below:
>
> Index: lib/Driver/Tools.cpp
> ===================================================================
> --- lib/Driver/Tools.cpp    (revision 193400)
> +++ lib/Driver/Tools.cpp    (working copy)
> @@ -2802,6 +2802,12 @@
>      CmdArgs.push_back(A->getValue());
>    }
>
> +  if (Arg *A = Args.getLastArg(options::OPT_foperatorarrow_depth_,
> +                               options::OPT_foperatorarrow_depth_EQ)) {
> +    CmdArgs.push_back("-foperatorarrow-depth");
> +    CmdArgs.push_back(A->getValue());
> +  }
> +
>    if (Arg *A = Args.getLastArg(options::OPT_fconstexpr_depth_EQ)) {
>      CmdArgs.push_back("-fconstexpr-depth");
>      CmdArgs.push_back(A->getValue());
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp    (revision 193400)
> +++ lib/Frontend/CompilerInvocation.cpp    (working copy)
> @@ -1313,6 +1313,8 @@
>    Opts.MathErrno = !Opts.OpenCL && Args.hasArg(OPT_fmath_errno);
>    Opts.InstantiationDepth =
>        getLastArgIntValue(Args, OPT_ftemplate_depth, 256, Diags);
> +  Opts.ArrowDepth =
> +      getLastArgIntValue(Args, OPT_foperatorarrow_depth, 256, Diags);
>    Opts.ConstexprCallDepth =
>        getLastArgIntValue(Args, OPT_fconstexpr_depth, 512, Diags);
>    Opts.ConstexprStepLimit =
> Index: lib/Sema/SemaExprCXX.cpp
> ===================================================================
> --- lib/Sema/SemaExprCXX.cpp    (revision 193400)
> +++ lib/Sema/SemaExprCXX.cpp    (working copy)
> @@ -5188,8 +5188,19 @@
>      llvm::SmallPtrSet<CanQualType,8> CTypes;
>      SmallVector<SourceLocation, 8> Locations;
>      CTypes.insert(Context.getCanonicalType(BaseType));
> +
> +    static unsigned int arrow_instantiation_count = 0;
> +    while (BaseType->isRecordType()) {
> +      arrow_instantiation_count++;
> +      if (arrow_instantiation_count >= getLangOpts().ArrowDepth) {
> +        Diag(OpLoc,diag::err_arrow_instantiation_depth_exceeded)
> +        << getLangOpts().ArrowDepth
> +        << Base->getSourceRange();
> +        Diag(OpLoc, diag::note_arrow_instantiation_depth)
> +        << getLangOpts().ArrowDepth;
> +    return ExprError();
> +      }
>
> -    while (BaseType->isRecordType()) {
>        Result = BuildOverloadedArrowExpr(
>            S, Base, OpLoc,
>            // When in a template specialization and on the first loop
> iteration,
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td    (revision 193400)
> +++ include/clang/Basic/DiagnosticSemaKinds.td    (working copy)
> @@ -2561,6 +2561,11 @@
>  // can handle that case properly.
>  def note_ovl_candidate_non_deduced_mismatch_qualified : Note<
>      "candidate template ignored: could not match %q0 against %q1">;
> +def err_arrow_instantiation_depth_exceeded : Error<
> +  "arrow operator instantiation exceeded maximum depth of %0">,
> +  DefaultFatal, NoSFINAE;
> +def note_arrow_instantiation_depth : Note<
> +  "use -foperatorarrow-depth=N to increase arrow operator instantiation
> depth">;
>
>  // Note that we don't treat templates differently for this diagnostic.
>  def note_ovl_candidate_arity : Note<"candidate "
> Index: include/clang/Basic/LangOptions.def
> ===================================================================
> --- include/clang/Basic/LangOptions.def    (revision 193400)
> +++ include/clang/Basic/LangOptions.def    (working copy)
> @@ -160,6 +160,8 @@
>  ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2,
> SOB_Undefined,
>               "signed integer overflow handling")
>
> +BENIGN_LANGOPT(ArrowDepth, 32, 256,
> +               "maximum arrow instantiation depth")
>  BENIGN_LANGOPT(InstantiationDepth, 32, 256,
>                 "maximum template instantiation depth")
>  BENIGN_LANGOPT(ConstexprCallDepth, 32, 512,
> Index: include/clang/Driver/CC1Options.td
> ===================================================================
> --- include/clang/Driver/CC1Options.td    (revision 193400)
> +++ include/clang/Driver/CC1Options.td    (working copy)
> @@ -442,6 +442,8 @@
>    HelpText<"Default type visibility">;
>  def ftemplate_depth : Separate<["-"], "ftemplate-depth">,
>    HelpText<"Maximum depth of recursive template instantiation">;
> +def foperatorarrow_depth : Separate<["-"], "foperatorarrow-depth">,
> +  HelpText<"Maximum depth of arrow operator instantiation">;
>  def fconstexpr_depth : Separate<["-"], "fconstexpr-depth">,
>    HelpText<"Maximum depth of recursive constexpr function calls">;
>  def fconstexpr_steps : Separate<["-"], "fconstexpr-steps">,
> Index: include/clang/Driver/Options.td
> ===================================================================
> --- include/clang/Driver/Options.td    (revision 193400)
> +++ include/clang/Driver/Options.td    (working copy)
> @@ -775,6 +775,8 @@
>  def ftemplate_depth_ : Joined<["-"], "ftemplate-depth-">, Group<f_Group>;
>  def ftemplate_backtrace_limit_EQ : Joined<["-"],
> "ftemplate-backtrace-limit=">,
>                                     Group<f_Group>;
> +def foperatorarrow_depth_EQ : Joined<["-"], "foperatorarrow-depth=">,
> Group<f_Group>;
> +def foperatorarrow_depth_ : Joined<["-"], "foperatorarrow-depth-">,
> Group<f_Group>;
>  def ftest_coverage : Flag<["-"], "ftest-coverage">, Group<f_Group>;
>  def fvectorize : Flag<["-"], "fvectorize">, Group<f_Group>,
>    HelpText<"Enable the loop vectorization passes">;
>
>
>
> Please if someone could help me by reviewing it and suggesting improvements?
>
> Thanks,
> Rahul
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>