[PATCH] D38846: [ELF] - Linkerscript: Fix issues with SUBALIGN.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 16 01:49:23 PDT 2017


grimar added a comment.

In https://reviews.llvm.org/D38846#898203, @ruiu wrote:

> I'd think you are overthinking. I don't think it is a good idea to add that many parameters to various functions just to check for a misuse of some rarely used feature.


The only function where I added parameter was `getSymbolValue`. It should be enough to control availability of `Dot` for all possible cases I think. 
One of pros also is that after such change it stops using 'Ctx', but uses 'CanUseDot' flag what removes dependency on 'Ctx' and improves readability.

> We should report an error instead of firing an assertion for SUBALIGN expression 0,

Reporting a error would not only be inconsistent with ld.bfd and rest LLD code (we change 0->1 with use of `std::max((uint64_t)1, E().getValue())` when parsing ALIGN and DATA_SEGMENT_ALIGN currently, but also incorrect, because ELF spec explicitly says:
"Some sections have address alignment constraints. <skipped> . Currently, only 0 and positive integral powers of two are allowed. Values 0 and 1 mean the section has no alignment constraints." (https://docs.oracle.com/cd/E19683-01/817-3677/chapter6-94076/index.html)
I see nothing wrong to have alignment 0 in script expression, we should treat them as 1, that is consistent with spec.


https://reviews.llvm.org/D38846





More information about the llvm-commits mailing list