[PATCH v2] bugfix: asserts being used to validate command line input.

[Richard Smith]

On Sun, Jun 8, 2014 at 2:55 PM, Larry D'Anna <larry at elder-gods.org> wrote:

> Bad inputs for -mfloat-abi and -mrelocation-model will cause asserts to
> fail.
> -mcode-model is checked earlier, but I added an error message for it as
> well for
> consistency's sake.
> ---
>
>  PATCH v2 changes: fixed whitespace.
>
>  include/clang/Basic/DiagnosticFrontendKinds.td |  6 ++++++
>  lib/CodeGen/BackendUtil.cpp                    | 21 ++++++++++++++++-----
>  test/Misc/float-abi.c                          |  3 +++
>  test/Misc/relocation-model.c                   |  3 +++
>  4 files changed, 28 insertions(+), 5 deletions(-)
>  create mode 100644 test/Misc/float-abi.c
>  create mode 100644 test/Misc/relocation-model.c
>
> diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td
> b/include/clang/Basic/DiagnosticFrontendKinds.td
> index 3527e8e..eac98e3 100644
> --- a/include/clang/Basic/DiagnosticFrontendKinds.td
> +++ b/include/clang/Basic/DiagnosticFrontendKinds.td
> @@ -67,6 +67,12 @@ def err_fe_unable_to_load_plugin : Error<
>      "unable to load plugin '%0': '%1'">;
>  def err_fe_unable_to_create_target : Error<
>      "unable to create target: '%0'">;
> +def err_fe_invalid_code_model : Error<
> +    "invalid code model: '%0'">;
> +def err_fe_invalid_relocation_model : Error<
> +    "invalid relocation model: '%0'">;
> +def err_fe_invalid_float_abi : Error<
> +    "invalid float abi: '%0'">;
>  def err_fe_unable_to_interface_with_target : Error<
>      "unable to interface with target machine">;
>  def err_fe_unable_to_open_output : Error<
> diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp
> index e15d357..c48f570 100644
> --- a/lib/CodeGen/BackendUtil.cpp
> +++ b/lib/CodeGen/BackendUtil.cpp
> @@ -373,7 +373,11 @@ TargetMachine
> *EmitAssemblyHelper::CreateTargetMachine(bool MustCreateTM) {
>        .Case("large", llvm::CodeModel::Large)
>        .Case("default", llvm::CodeModel::Default)
>        .Default(~0u);
> -  assert(CodeModel != ~0u && "invalid code model!");
> +  if (CodeModel == ~0u) {
> +    if (MustCreateTM)
> +      Diags.Report(diag::err_fe_invalid_code_model) <<
> CodeGenOpts.CodeModel;
> +    return nullptr;
> +  }
>    llvm::CodeModel::Model CM =
> static_cast<llvm::CodeModel::Model>(CodeModel);
>
>    SmallVector<const char *, 16> BackendArgs;
> @@ -411,10 +415,13 @@ TargetMachine
> *EmitAssemblyHelper::CreateTargetMachine(bool MustCreateTM) {
>      RM = llvm::Reloc::Static;
>    } else if (CodeGenOpts.RelocationModel == "pic") {
>      RM = llvm::Reloc::PIC_;
> -  } else {
> -    assert(CodeGenOpts.RelocationModel == "dynamic-no-pic" &&
> -           "Invalid PIC model!");
> +  } else if (CodeGenOpts.RelocationModel == "dynamic-no-pic") {
>      RM = llvm::Reloc::DynamicNoPIC;
> +  } else {
> +    if (MustCreateTM)
> +      Diags.Report(diag::err_fe_invalid_relocation_model)
> +             << CodeGenOpts.RelocationModel;
> +    return nullptr;
>    }
>
>    CodeGenOpt::Level OptLevel = CodeGenOpt::Default;
> @@ -450,7 +457,11 @@ TargetMachine
> *EmitAssemblyHelper::CreateTargetMachine(bool MustCreateTM) {
>    else if (CodeGenOpts.FloatABI == "hard")
>      Options.FloatABIType = llvm::FloatABI::Hard;
>    else {
> -    assert(CodeGenOpts.FloatABI.empty() && "Invalid float abi!");
> +    if (!CodeGenOpts.FloatABI.empty()) {
> +      if (MustCreateTM)
> +       Diags.Report(diag::err_fe_invalid_float_abi) <<
> CodeGenOpts.FloatABI;
> +      return nullptr;
> +    }
>      Options.FloatABIType = llvm::FloatABI::Default;
>    }
>
> diff --git a/test/Misc/float-abi.c b/test/Misc/float-abi.c
> new file mode 100644
> index 0000000..7201cbe0
> --- /dev/null
> +++ b/test/Misc/float-abi.c
> @@ -0,0 +1,3 @@
> +// RUN: not %clang_cc1 -S -mfloat-abi notanabi %s 2>&1 | FileCheck
> -check-prefix CHECK-INVALID %s
> +
> +// CHECK-INVALID: error: invalid float abi: 'notanabi'
> diff --git a/test/Misc/relocation-model.c b/test/Misc/relocation-model.c
> new file mode 100644
> index 0000000..e49f1bc
> --- /dev/null
> +++ b/test/Misc/relocation-model.c
> @@ -0,0 +1,3 @@
> +// RUN: not %clang_cc1 -S -mrelocation-model foobar %s 2>&1 | FileCheck
> -check-prefix CHECK-INVALID %s
> +
> +// CHECK-INVALID: error: invalid relocation model: 'foobar'
> --
> 1.8.3.2
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>

Please fold these two tests together into one test in test/Driver (maybe
invalid-target.c?). Please also add a test for the code model diagnostic.

Other than that, this looks good to me, thanks! Do you need someone to
commit this for you?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140624/c14f1017/attachment.html>