[PATCH] D43069: [ELF] - Report error if removed empty output section declaration used undefined symbols.

[Rafael Avila de Espindola via llvm-commits]

Given how simple the patch now is and given that it avoids us producing
an executable with an undefined symbol, I would suggest committing it.

Any wording change can be done in post commit review.

Cheers,
Rafael

George Rimar <grimar at accesssoftek.com> writes:

> Rui, do you have any objections ?
>
> Best regards,
> George | Developer | Access Softek, Inc
>
> ________________________________________
> От: Rafael Avila de Espindola <rafael.espindola at gmail.com>
> Отправлено: 21 февраля 2018 г. 3:04
> Кому: reviews+D43069+public+fe445c195a6cf9c5 at reviews.llvm.org; George Rimar; ruiu at google.com
> Копия: arichardson.kde at gmail.com; emaste at freebsd.org; Evgeny Leviant; Igor Kudrin; llvm-commits at lists.llvm.org
> Тема: Re: [PATCH] D43069: [ELF] - Report error if removed empty output section declaration used undefined symbols.
>
> Still LGTM.
>
> The current situation (producing an undefined symbol and no error) is
> bogus and I can't think of a simpler way to fix this.
>
> Cheers,
> Rafael
>
> George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
>
>> grimar updated this revision to Diff 134856.
>> grimar added a comment.
>>
>> - Comment update. NFC.
>>
>>
>> https://reviews.llvm.org/D43069
>>
>> Files:
>>   ELF/LinkerScript.cpp
>>   test/ELF/linkerscript/address-expr-symbols.s
>>
>>
>> Index: test/ELF/linkerscript/address-expr-symbols.s
>> ===================================================================
>> --- test/ELF/linkerscript/address-expr-symbols.s
>> +++ test/ELF/linkerscript/address-expr-symbols.s
>> @@ -0,0 +1,15 @@
>> +# REQUIRES: x86
>> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
>> +
>> +# RUN: echo "SECTIONS { .bar (foo) : { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> +# CHECK: symbol not found: foo
>> +
>> +# RUN: echo "SECTIONS { .bar : AT(foo) { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> +
>> +# RUN: echo "SECTIONS { .bar : ALIGN(foo) { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> +
>> +# RUN: echo "SECTIONS { .bar : SUBALIGN(foo) { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> Index: ELF/LinkerScript.cpp
>> ===================================================================
>> --- ELF/LinkerScript.cpp
>> +++ ELF/LinkerScript.cpp
>> @@ -767,6 +767,13 @@
>>  }
>>
>>  static bool isAllSectionDescription(const OutputSection &Cmd) {
>> +  // We do not want to remove sections that have custom address or align
>> +  // expressions set even if them are empty. We keep them because we
>> +  // want to be sure that any expressions can be evaluated and report
>> +  // an error otherwise.
>> +  if (Cmd.AddrExpr || Cmd.AlignExpr || Cmd.LMAExpr)
>> +    return false;
>> +
>>    for (BaseCommand *Base : Cmd.SectionCommands)
>>      if (!isa<InputSectionDescription>(*Base))
>>        return false;
>>
>>
>> Index: test/ELF/linkerscript/address-expr-symbols.s
>> ===================================================================
>> --- test/ELF/linkerscript/address-expr-symbols.s
>> +++ test/ELF/linkerscript/address-expr-symbols.s
>> @@ -0,0 +1,15 @@
>> +# REQUIRES: x86
>> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
>> +
>> +# RUN: echo "SECTIONS { .bar (foo) : { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> +# CHECK: symbol not found: foo
>> +
>> +# RUN: echo "SECTIONS { .bar : AT(foo) { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> +
>> +# RUN: echo "SECTIONS { .bar : ALIGN(foo) { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> +
>> +# RUN: echo "SECTIONS { .bar : SUBALIGN(foo) { } };" > %t.script
>> +# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
>> Index: ELF/LinkerScript.cpp
>> ===================================================================
>> --- ELF/LinkerScript.cpp
>> +++ ELF/LinkerScript.cpp
>> @@ -767,6 +767,13 @@
>>  }
>>
>>  static bool isAllSectionDescription(const OutputSection &Cmd) {
>> +  // We do not want to remove sections that have custom address or align
>> +  // expressions set even if them are empty. We keep them because we
>> +  // want to be sure that any expressions can be evaluated and report
>> +  // an error otherwise.
>> +  if (Cmd.AddrExpr || Cmd.AlignExpr || Cmd.LMAExpr)
>> +    return false;
>> +
>>    for (BaseCommand *Base : Cmd.SectionCommands)
>>      if (!isa<InputSectionDescription>(*Base))
>>        return false;