[PATCH] D34234: [ELF] Emit only one error if -z max-page-size without value

[Rafael Avila de Espindola via llvm-commits]

LGTM

James Henderson via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> jhenderson created this revision.
> Herald added a subscriber: emaste.
>
> In  https://reviews.llvm.org/rL305364, @ruiu changed the mechanism that parses -z option values slightly. This caused a bug, as demonstrated by this test, which now fails:
>
>   # REQUIRES: x86
>   # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
>   # RUN: ld.lld %t.o -o %t -z max-page-size
>   
>   .global _start
>   _start:
>     nop
>
> Before, the link succeeded and set the max-page-size to the target default.
>
> After we get the following two error messages:
> "invalid max-page-size: "
> "max-page-size: value isn't a power of 2" (because an uninitialised variable ends up being passed back to getMaxPageSize).
>
> I think we should have the first error, but not the second, as demonstrated by the new test in this change.
>
>
> https://reviews.llvm.org/D34234
>
> Files:
>   ELF/Driver.cpp
>   test/ELF/invalid-z.s
>
>
> Index: test/ELF/invalid-z.s
> ===================================================================
> --- test/ELF/invalid-z.s
> +++ test/ELF/invalid-z.s
> @@ -0,0 +1,9 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: not ld.lld %t.o -o %t -z max-page-size 2>&1 | FileCheck %s
> +# CHECK: invalid max-page-size
> +# CHECK-NOT: error
> +
> +.global _start
> +_start:
> +  nop
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -301,7 +301,7 @@
>    for (auto *Arg : Args.filtered(OPT_z)) {
>      std::pair<StringRef, StringRef> KV = StringRef(Arg->getValue()).split('=');
>      if (KV.first == Key) {
> -      uint64_t Result;
> +      uint64_t Result = Default;
>        if (!to_integer(KV.second, Result))
>          error("invalid " + Key + ": " + KV.second);
>        return Result;
>
>
> Index: test/ELF/invalid-z.s
> ===================================================================
> --- test/ELF/invalid-z.s
> +++ test/ELF/invalid-z.s
> @@ -0,0 +1,9 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: not ld.lld %t.o -o %t -z max-page-size 2>&1 | FileCheck %s
> +# CHECK: invalid max-page-size
> +# CHECK-NOT: error
> +
> +.global _start
> +_start:
> +  nop
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -301,7 +301,7 @@
>    for (auto *Arg : Args.filtered(OPT_z)) {
>      std::pair<StringRef, StringRef> KV = StringRef(Arg->getValue()).split('=');
>      if (KV.first == Key) {
> -      uint64_t Result;
> +      uint64_t Result = Default;
>        if (!to_integer(KV.second, Result))
>          error("invalid " + Key + ": " + KV.second);
>        return Result;
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits