[PATCH] D35942: [ELF] - Do not crash when ALIGN/DATA_SEGMENT_ALIGN expression used with zero value.

[Rafael Avila de Espindola via llvm-commits]

LGTM

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
> Herald added a subscriber: emaste.
>
> Previously we would crash,
> I suggest to use 1 value instead 0 in this case.
> This looks to be consistent with GNU linkers
> and reasonable and simple behavior itself.
>
>
> https://reviews.llvm.org/D35942
>
> Files:
>   ELF/ScriptParser.cpp
>   test/ELF/linkerscript/align.s
>   test/ELF/linkerscript/operators.s
>
>
> Index: test/ELF/linkerscript/operators.s
> ===================================================================
> --- test/ELF/linkerscript/operators.s
> +++ test/ELF/linkerscript/operators.s
> @@ -25,6 +25,7 @@
>  # RUN:  commonpagesize = CONSTANT (COMMONPAGESIZE); \
>  # RUN:  . = 0xfff0; \
>  # RUN:  datasegmentalign = DATA_SEGMENT_ALIGN (0xffff, 0); \
> +# RUN:  datasegmentalign2 = DATA_SEGMENT_ALIGN (0, 0); \
>  # RUN: }" > %t.script
>  # RUN: ld.lld %t --script %t.script -o %t2
>  # RUN: llvm-objdump -t %t2 | FileCheck %s
> @@ -51,6 +52,7 @@
>  # CHECK: 00000000001000 *ABS* 00000000 maxpagesize
>  # CHECK: 00000000001000 *ABS* 00000000 commonpagesize
>  # CHECK: 0000000000ffff *ABS* 00000000 datasegmentalign
> +# CHECK: 0000000000fff0 *ABS* 00000000 datasegmentalign2
>  
>  ## Mailformed number error.
>  # RUN: echo "SECTIONS { . = 0x12Q41; }" > %t.script
> Index: test/ELF/linkerscript/align.s
> ===================================================================
> --- test/ELF/linkerscript/align.s
> +++ test/ELF/linkerscript/align.s
> @@ -66,6 +66,21 @@
>  # SYMBOLS-NEXT: 0000000000011000         .bbb            00000000 __start_bbb
>  # SYMBOLS-NEXT: 0000000000012000         .bbb            00000000 __end_bbb
>  
> +## Check that ALIGN zero do nothing and does not crash #1.
> +# RUN: echo "SECTIONS { . = ALIGN(0x123, 0); .aaa : { *(.aaa) } }" > %t.script
> +# RUN: ld.lld -o %t4 --script %t.script %t
> +# RUN: llvm-objdump -section-headers %t4 | FileCheck %s -check-prefix=ZERO
> +
> +# ZERO:      Sections:
> +# ZERO-NEXT: Idx Name          Size      Address         Type
> +# ZERO-NEXT:   0               00000000 0000000000000000
> +# ZERO-NEXT:   1 .aaa          00000008 0000000000000123 DATA
> +
> +## Check that ALIGN zero do nothing and does not crash #2.
> +# RUN: echo "SECTIONS { . = 0x123; . = ALIGN(0); .aaa : { *(.aaa) } }" > %t.script
> +# RUN: ld.lld -o %t5 --script %t.script %t
> +# RUN: llvm-objdump -section-headers %t5 | FileCheck %s -check-prefix=ZERO
> +
>  .global _start
>  _start:
>   nop
> Index: ELF/ScriptParser.cpp
> ===================================================================
> --- ELF/ScriptParser.cpp
> +++ ELF/ScriptParser.cpp
> @@ -904,13 +904,15 @@
>      expect("(");
>      Expr E = readExpr();
>      if (consume(")"))
> -      return [=] { return alignTo(Script->getDot(), E().getValue()); };
> +      return [=] {
> +        return alignTo(Script->getDot(), std::max((uint64_t)1, E().getValue()));
> +      };
>      expect(",");
>      Expr E2 = readExpr();
>      expect(")");
>      return [=] {
>        ExprValue V = E();
> -      V.Alignment = E2().getValue();
> +      V.Alignment = std::max((uint64_t)1, E2().getValue());
>        return V;
>      };
>    }
> @@ -931,7 +933,9 @@
>      expect(",");
>      readExpr();
>      expect(")");
> -    return [=] { return alignTo(Script->getDot(), E().getValue()); };
> +    return [=] {
> +      return alignTo(Script->getDot(), std::max((uint64_t)1, E().getValue()));
> +    };
>    }
>    if (Tok == "DATA_SEGMENT_END") {
>      expect("(");
>
>
> Index: test/ELF/linkerscript/operators.s
> ===================================================================
> --- test/ELF/linkerscript/operators.s
> +++ test/ELF/linkerscript/operators.s
> @@ -25,6 +25,7 @@
>  # RUN:  commonpagesize = CONSTANT (COMMONPAGESIZE); \
>  # RUN:  . = 0xfff0; \
>  # RUN:  datasegmentalign = DATA_SEGMENT_ALIGN (0xffff, 0); \
> +# RUN:  datasegmentalign2 = DATA_SEGMENT_ALIGN (0, 0); \
>  # RUN: }" > %t.script
>  # RUN: ld.lld %t --script %t.script -o %t2
>  # RUN: llvm-objdump -t %t2 | FileCheck %s
> @@ -51,6 +52,7 @@
>  # CHECK: 00000000001000 *ABS* 00000000 maxpagesize
>  # CHECK: 00000000001000 *ABS* 00000000 commonpagesize
>  # CHECK: 0000000000ffff *ABS* 00000000 datasegmentalign
> +# CHECK: 0000000000fff0 *ABS* 00000000 datasegmentalign2
>  
>  ## Mailformed number error.
>  # RUN: echo "SECTIONS { . = 0x12Q41; }" > %t.script
> Index: test/ELF/linkerscript/align.s
> ===================================================================
> --- test/ELF/linkerscript/align.s
> +++ test/ELF/linkerscript/align.s
> @@ -66,6 +66,21 @@
>  # SYMBOLS-NEXT: 0000000000011000         .bbb            00000000 __start_bbb
>  # SYMBOLS-NEXT: 0000000000012000         .bbb            00000000 __end_bbb
>  
> +## Check that ALIGN zero do nothing and does not crash #1.
> +# RUN: echo "SECTIONS { . = ALIGN(0x123, 0); .aaa : { *(.aaa) } }" > %t.script
> +# RUN: ld.lld -o %t4 --script %t.script %t
> +# RUN: llvm-objdump -section-headers %t4 | FileCheck %s -check-prefix=ZERO
> +
> +# ZERO:      Sections:
> +# ZERO-NEXT: Idx Name          Size      Address         Type
> +# ZERO-NEXT:   0               00000000 0000000000000000
> +# ZERO-NEXT:   1 .aaa          00000008 0000000000000123 DATA
> +
> +## Check that ALIGN zero do nothing and does not crash #2.
> +# RUN: echo "SECTIONS { . = 0x123; . = ALIGN(0); .aaa : { *(.aaa) } }" > %t.script
> +# RUN: ld.lld -o %t5 --script %t.script %t
> +# RUN: llvm-objdump -section-headers %t5 | FileCheck %s -check-prefix=ZERO
> +
>  .global _start
>  _start:
>   nop
> Index: ELF/ScriptParser.cpp
> ===================================================================
> --- ELF/ScriptParser.cpp
> +++ ELF/ScriptParser.cpp
> @@ -904,13 +904,15 @@
>      expect("(");
>      Expr E = readExpr();
>      if (consume(")"))
> -      return [=] { return alignTo(Script->getDot(), E().getValue()); };
> +      return [=] {
> +        return alignTo(Script->getDot(), std::max((uint64_t)1, E().getValue()));
> +      };
>      expect(",");
>      Expr E2 = readExpr();
>      expect(")");
>      return [=] {
>        ExprValue V = E();
> -      V.Alignment = E2().getValue();
> +      V.Alignment = std::max((uint64_t)1, E2().getValue());
>        return V;
>      };
>    }
> @@ -931,7 +933,9 @@
>      expect(",");
>      readExpr();
>      expect(")");
> -    return [=] { return alignTo(Script->getDot(), E().getValue()); };
> +    return [=] {
> +      return alignTo(Script->getDot(), std::max((uint64_t)1, E().getValue()));
> +    };
>    }
>    if (Tok == "DATA_SEGMENT_END") {
>      expect("(");